In our Google-internal build systems, with the strictest compiler
settings, we were seeing errors related to these two issues:
1. A missing require
2. Incomplete type info
This fixes the errors in that environment.
Change-Id: I9c4c482ed6ef8f669e113263a70958b53138c4ee
This fixes all the license headers in the main library, which corrects
the appearance of the main license in the compiled output.
It seems that the `!` in the header forces the compiler to keep it in
the output. I believe older compiler releases did this purely based
on `@license`.
Issue #2638
Change-Id: I7f0e918caad10c9af689c9d07672b7fe9be7b2f3
This adds our first screenshot-based layout tests and the
infrastructure to use WebDriver for screenshots through Karma.
This new kind of test will be skipped in any non-WebDriver context.
There are many pieces to this system.
First, we update the Karma WebDriver launcher to a newly-released
version that lets us access the client spec object from the launcher.
Second, we build a Karma middleware plugin to respond to HTTP requests
from the tests. We handle /screenshot/isSupported and return a bool
so tests can be skipped on non-WebDriver launchers. We also handle
/screenshot/diff to take the screenshot and compare it to a known-good
version.
The screenshot is a full-page screenshot, since element screenshots
don't work consistently across all the browsers in our test lab. The
screenshot is then cropped to a rectangle specified in the request.
This rectangle is measured to match a specific element, so in
practice, we are screenshotting just one element.
Browsers use sub-pixel rendering, effectively rendering at a scale
larger than the "pixels" seen by JS. The screenshot comes in at this
scale, so the requested cropping rectangle is scaled to match, then
the cropped screenshot is scaled down to the JS-measured size.
Because of sub-pixel rendering, element offsets can be non-integer
numbers. Normally, Karma puts the tests in a iframe, above which is a
variable-height banner showing which devices are connected to Karma
and what state they are in. So this variation and the lack of integer
offsets means we can run into stability issues due to rounding errors.
To make offsets consistent and improve stability of the screenshots,
this banner is now disabled in our Karma config.
The cropping, scaling, and diffing of images is handled Karma-side by
a node module called Jimp.
Before we start the layout tests for UITextDisplayer, we use a node
module in the browser called fontfaceonload to wait for our web fonts
to be fully loaded. This module is a polyfill that polls on IE and
uses a standard API in modern browsers to wait for our font to load.
This is all wrapped into a new test util called waitForFont.
Screenshots are stored in test/test/assets/screenshots/ and are
organized into folders by platform and browser and named according to
an identifier specified by each test case. The new screenshot is
written to disk with the suffix "-new", and a diff image is written
with the suffix "-diff". When a test fails, we can review the changes
in a browser with test/test/assets/screenshots/review.html. The
known-good screenshots can be updated with the new tool
build/updateScreenshots.py.
Change-Id: Ib477fd3c459de466c6dc91e9a60d3e2579164b12
While developing screenshot-based layout tests, it was discovered that
IE 11 did not position VTT/TTML regions in the same way as other
browsers. This turned out to be caused by a bug in IE's flexbox
implementation in which flex items that are center-positioned can
overflow the flex parent.
This uses the work-around recommended here:
https://github.com/philipwalton/flexbugs/tree/6e720da8#flexbug-2Closes#2584
Change-Id: I22b9790b8f5798ba757a111d2c502dd5a3415441
The margin: auto style centers the element both horizontally and
vertically. We don't want vertical centering, and we already get
horizontal centering when appropriate through other styles. So
removing margin: auto is the best fix.
Closes#2524
Change-Id: Icff74e49b3917b50141d1205232ebb0856fbcf16
The position: 'absolute' style should only be set when other
positioning attributes (left, right, top, bottom) are being set.
Otherwise, the natural layout flow is disturbed, without anything to
replace it.
Closes#2524
Change-Id: I4062c68253218a0f4ace6560a8847381d80c48d5
When no specific size has been set on a Cue object, the default will
now be 0, which means "auto" size to the text displayer. If a cue has
no specific width, the background will fit to the text. If a cue has
an explicit width of 100%, the background will fill the entire
horizontal space of the text container.
This fixes the 100%-width background introduced in Change-Id
I4500b8f637f3f43f48d019d14ef527e75d960fec in #2524.
Fixes#2524
Change-Id: I98caeb89d3da640a8175560810c8ccf2de27a2bb
In player, SimpleTextDisplayer was set as the default TextDisplayer. In
our UI, it changes the configuration to use the UITextDisplayer, and
UITextDisplayer gets constructed with the player. Later in our demo, it
resets the config, so an extra SimpleTextDisplayer gets constructed.
This introduces an extra TextTrack created by SimpleTextDisplayer.
This change sets the UITextDisplayer as the default TextDisplayer in
player's default config.
Closes#2516
Change-Id: I3f653be9fad8b2edbc2fb9de84e8abb327dcfc51
When SimpleTextDisplayer is constructed, a TextTrack is created,
and that cannot be removed. This introduces an extra TextTrack when the
content is played in src= mode.
This is the straight-forward solution to filter the extra TextTrack out
by the label in src= mode.
Issue #2516
Change-Id: I8c417f837e4ad2f90ec779b533c8484de9e22b11
We used to alias static utility methods by assigning the method itself
to a local variable. This is not allowed by the new Closure Compiler
release that we are adopting, and for good reason.
The old compiler did not understand the use of "this" in static
methods, but the new one does. And it turns out that when we start
using "this" in static methods (see #2532), aliasing the method itself
can break everything.
When you refer to "this" in a static method, it refers to the class.
This is really useful in utility classes that have private static
methods they use to perform common tasks. However, just as it ruins
the value of "this" to alias an instance method, the same is true of
an ES6 class's static method.
The fix is to always alias the class instead of the method. The new
compiler will simply not let us get away with the old way any more, so
regressions after this are unlikely.
Issue #2528 (compiler upgrade)
Issue #2532 (static "this")
Change-Id: Id800d466e639c7cbcf4aa6fbb05114c772a2229f
The old compiler would let us export a static method on a class
without exporting the whole class and its constructor. The new
compiler silently ignores `@export` for any methods of a non-exported
class.
This means that for the latest Closure Compiler, we must export any
class with exported static methods. In some cases, these are
non-utility classes with constructors we'd rather not export, but the
constructor is implicitly exported by exporting the class itself.
This was initially caught by the integration tests. The error wasn't
especially helpful, so I added a try/catch to loadShaka that makes the
error more apparent:
TypeError: Cannot read property 'registerParserByMime' of undefined
To make sure we do not make this mistake again, I've added a check to
the extern generator, which was already able to detect these types of
classes. I don't know a compiler-based way to do it, since the
compiler silently ignores the export annotations in these cases.
Issue #2528
Change-Id: I797c75a8098b0bb3cf837588569f878253dec2cf
Various issues with the nullability of string types led to various
fixes, including:
- adding an assertion or runtime check that something is not null
- moving an existing null check to before a calculation
- converting a test expectation into an assertion that the compiler
understands (which will still fail the test if the assertion
fails)
These issues were caught by a compiler upgrade.
Issue #2528
Change-Id: I11da091c9e7974c8bea84b3b584cbd29d1e320e2
This partially reverts commit ad3d4604, because that fix seems to have
caused #2523.
This also adds a regression test for #2523.
Reopens#2516Closes#2523
Change-Id: If3ed5942fff029f522e24048edcb4a04e7cc30e9
When the non-UI TextDisplayer is constructed, it creates an extra
TextTrack that can never be removed. This leads to a bogus text track
showing up in the UI when content is played in src= mode.
This changes the TextDisplayer to disable the extra TextTrack (since
there is no API to remove it) and changes the Player to ignore
disabled TextTracks when generating a track list for src= playbacks.
Closes#2516
Change-Id: I2e651f737445049da5fa46a798a2bc0751de2822
A regex in the TTML parser was looking for "word" characters (\w),
which is short-hand for non-whitespace ASCII characters, and excludes
Unicode characters and symbols from non-English languages.
There is a better option in Regex in some browsers, which is /\p{L}/u.
This is not supported universally yet, and defining an equivalent
regex constant using character codes is over 4kB of extra code.
It seems that the original author of that code meant "word" (\w) in
the sense of "non-whitespace" (\S), so we can just use \S instead.
This change also adds a regression test that doesn't depend on the
specifics of the regular expression used.
Closes#2478
Change-Id: I794258a797ba5d26a7bd8fc0a5244adc70c8a1ef
querySelector isn't available in Shaka Player Embedded. Since we only
use it to find nodes with a specific tag name, we can usually use
getElementsByTagName.
Issue google/shaka-player-embedded#113
Change-Id: Ia225e5d1f7598b13bd05db868fe2ea566dca4493
Instead of having the "factories" use "new" to construct them, now they
will be plain functions.
Closes#1521
Change-Id: Ia6151ad679a78a5c6db128d43094c82add0af348
The appendCues method in TextEngine was completely unused. Another
method was exported that should not have been.
Change-Id: Icee35d8ccbc5a903cb13409211a8c5770f1c6a87
This changes the eslint rule to enforce a strict pattern for the
argument comments. The comment must appear before the argument and
must be /* foo= */. This still ignores line comments.
Change-Id: I3afb01c65e1088eda13facb3aeeaa7595a2f5aee
This reflects changes in Google's policy on JavaScript license
headers, which should be smaller to avoid increasing the size of the
binary unnecessarily.
This also updates the company name from "Google, Inc" to "Google LLC".
Change-Id: I3f8b9ed3700b6351f43173d50c94d35c333e82b4
Previously, if no XML was produced, or if it failed to have a <tt> element,
we assumed that parsing had failed. This caught some failures, but not
every possible failure; there are some partial failures that still return
valid XML. These sorts of errors are signaled by the output containing a
<parsererror> tag; this CL makes us also return an error in this case.
It also corrects the documentation for XML parsing errors, which was
incorrect; it seems that we had been confusing INVALID_XML and
DASH_INVALID_XML for documentation.
Issue #2157
Change-Id: I79716c9d2ff90ed9672eef8e54dd4019d8ca109b
Something I noticed while working on issue #2157; their text tracks
contained percent values with decimals in them (for example "4.17%"),
which was something the percentage-parsing regex we were using did
not handle.
Change-Id: I90f4a223f0a335057a92ae606e7298eefaa0b4aa
For VOD, the first segment of every Media Playlist in every Variant
Stream must start at the same media timestamp. Thus, we can get the
start time once and store the value, and all the streams can use the
value directly. For live, we assume for the same.
This change reduces calling getStartTime_() from 15 times to once for
Angel One, and also fixes the text segment timestamp issue.
Also removed associated methods that no longer needed.
Closes#1558Closes#1563
Change-Id: I0e95ab93ea2b13758128f11019b262bc53dbcd38
We would incorrectly initialize the embedded captions multiple times
during a Period transition, which caused duplicate cues to be given to
the displayer. We also wouldn't handle the case when switching between
embedded captions and external text during a Period transition. This
fixes both cases and adds tests for them.
This also avoids passing an empty cue list to the displayer.
Fixes#2076
Change-Id: I89add3eb86ad8d93644bba14eabd11f98d57bc5e
When using DOMParser parseFromString with an empty string, it returns an errored XML document. The next part of the code is then executed and throws an unnecessary error.
This adds a special case for an empty string.
These are intermittently used and cannot be verified. Most of our
methods can throw a shaka.util.Error, so having it doesn't really add
anything. Plus, if we change a function to throw, we'd need to update
all their callers to ensure they have an accurate description of what
they throw; otherwise we can't trust the directive.
Change-Id: I520bd0fc4c33443e967bf5b103ca5aa9e3274884
Using "new Uint8Array" with a TypedArray creates a copy of the buffer;
this is unnecessarily expensive for large buffers. This adds a rule
to disallow using it in favor of a new utility that correctly creates
a new "view" on the same buffer.
Note it is fine to pass an ArrayBuffer to "new Uint8Array" and it won't
copy; but there there are many cases where the type is BufferSource,
so it could be a TypedArray. Unfortunately, there are many other cases
where we explicitly pass an ArrayBuffer; but the compiler rules don't
allow us to whitelist this case (since ArrayBuffer is part of
BufferSource).
Change-Id: I58696a85a9cbcc188c0b16919c9eeb63e56edca1
Now the EbmlParser and DataViewReader types can accept BufferSource
objects to avoid the caller having to convert it at each callsite.
Change-Id: I527fbc638485214bfe976d7da89b79b6098033a8
This changes the network API to use BufferSource instead of ArrayBuffer,
which allows plugins to return a "view" on a buffer instead of the
whole buffer. This also adds some utilities for changing between
views and buffers.
Lastly this forbids the use of the "buffer" property of TypedArrays
since it doesn't work with partial "views". This audits and fixes the
usages of the "buffer" property to ensure correct usage.
It should be noted that both MSE and EME accept a BufferSource as input,
so we don't need to convert a "view" into an ArrayBuffer before passing
to it.
Change-Id: Iaa417773f8ce5304424e43c7372ce10ebf540d2a
It turns out that, on some browsers, incomplete XML does not properly
parse. This means the parseFirstCue tests did not work on those platforms.
This modifies those tests on platforms that do not support partial XML;
when no support is detected, it instead tries to get the first cue
from a non-partial segment.
This also modifies the HLS parser to only load partial text segments if
the text parser supports it; otherwise, the hls parser will load full
captions in order to extract start times.
b/137945635
Issue #2037
Change-Id: I58112864a818baf155b365a65fba1c8a56eb9520
Previously, when the text engine tried to load the start time of
a segment, it would parse every cue in that segment, then check the
time of the first cue. This was judged to not be a significant
performance issue, as parsing cues is a fast operation. However, it
did have an unintended side-effect: in some situations, this method
was being passed partial segments; notably, the HLS parser would load
the first 2048kb of the stream's texts to extract timing data.
If the caption parsers tried to actually parse an incomplete caption,
they would error.
This gives the text parsers "parseFirstCue" methods, and uses those
methods when it only needs the first cue anyway.
Fixes#2037
Change-Id: I2a1fb2f1a96d98967f0c6e6a5c277914a28b42ad
Muhammad Haris
Joey Parrish
Alvaro Velad
Alex Jones
michellezhuo
kumarashu123
Jacob Trimble
Tanya Gelahova
Tijn Porcelijn
Theodore Abshire
fadomire
Vincent Valot