LOG(FATAL) aborts the executable, which means gtest can't write any
reports it might be configured to write when tests are complete. This
will interfere with reporting.
This converts the only use of LOG(FATAL), which was in
ReadTestDataFile(), to LOG(ERROR). This also updates test cases to avoid
crashing when ReadTestDataFile() returns an empty buffer.
Our workflows and Dockerfiles now use Ninja on Linux & Mac, which
enables safe parallel builds. This significantly speeds up our
workflows.
GitHub Actions typical compilation times (build step only):
- Linux 19m => 9m
- macOS 23m => 8m
- Windows 12m => 10m
- Linux arm64 (self-hosted) 72m => 29m
- Docker build 25m => 14m
Overall workflow time: 84m => 33m
Compilation time on my workstation (12 CPUs @3.3GHz): 15m => 3m
This also adds a new environment variable "PACKAGER_LOW_MEMORY_BUILD".
If defined when CMake is first run, this will configure the build to
disable parallel linking to reduce memory usage. This helps us avoid
failures on our self-hosted arm64 machines, where 6 CPUs share 4GB of
RAM.
NOTE: Parallel builds are **NOT** recommended with Unix Makefiles due to
the use of excessive RAM during parallel linking. Unix Makefiles, unlike
Ninja, cannot be configured to restrict parallel linking during a
parallel build. Anecdotally, parallel builds with Makefiles have
exhausted a system with 32GB RAM. (My workstation.)
In a follow-up, I will update the build documentation to refer to CMake
and recommend all of the flags now used in our workflows.
CentOS 8 is EOL, and the official docker images will not be updated. See
https://www.centos.org/centos-linux-eol/
This switches to the latest release (CentOS 9 Stream). Since CentOS no
longer provides official images, this uses a third-party image provider
(https://hub.docker.com/u/tgagor)
This change also exposed a warning in absl that needed to be silenced in
the default version of GCC in CentOS 9 Stream (GCC 11.4).
The `__FILE__` macro does not always get you an absolute path, so our
assumptions about it in test_data_util.cc were flawed.
If `foo.c` references `__FILE__`, something like `gcc -c foo.c`, will
define `__FILE__` as `"foo.c"`. If you use `gcc -c /path/to/foo.c`, then
`__FILE__` will be `"/path/to/foo.c"`.
The Ninja generator for CMake only generates absolute paths for source
files in certain CMake versions. (Exact range unknown.) Rather than
require newer CMake versions and depend on CMake's Ninja generator
maintaining the latest behavior forever, set the macro TEST_DATA_DIR to
point to the test data folder. This is consistent and never depends on
unspecified behavior.
This change will allow the use of the Ninja generator with older
versions of CMake, as found in most of our Docker builds.
- Update protobuf to v23.4, to fix the cstdint error on Arch Linux, and
make some related changes:
- Silence additional compiler warnings for the new protobuf
- Update absl to 20230125.3, to support the protobuf update, and make
some related changes:
- Silence additional compiler warnings for the new protobuf
- Replace GOOGLE_CHECK_OK with ABSL_CHECK_OK
- Replace GUARDED_BY with ABSL_GUARDED_BY
- Update other instances of cstdint missing in our own code
- Always pull the latest docker images, to avoid stale results when your
workstation has pulled an older image at the same label
Rebasing #1148 on top of current cmake branch
Related to #1047
---------
Co-authored-by: Bartek Zdanowski <bartek.zdanowski@gmail.com>
Co-authored-by: Joey Parrish <joeyparrish@google.com>
All HTTP-based tests now use an embedded test server instead of
httpbin.org, which makes them much faster and more reliable.
These more reliable tests also exposed some issues that began recently
with PR #1201. HttpFile's Flush() semantics were different than those
documented for files in general. Flush() used to close the file for
uploading, so that no further writes were allowed, but the documentation
stated that it would only flush data to its destination. PR #1201
brought HttpFile's Flush() in line with the docs, but gave us no way to
terminate a chunked upload.
This adds a new method to File called CloseForWriting(), which
terminates a chunked upload for HttpFile. The only other implementation
that does anything is UdpFile, which uses the socket library function
shutdown() to terminate writes while allowing reads.
This also tweaks HttpFile::CloseWithStatus() so that it will not
generate an error if the file is closed before the HTTP response is
written to the download cache.
This modifies the test HttpFileTest.MultipleWrites so that the file is
Flushed after each chunk. This adds test coverage for the changes
introduced in PR #1201.
Fixes#1224 (missing test coverage for HttpFile::Flush)
This adds Mongoose as a third-party library, and builds on top of that
an embedded HTTP server for our unit tests.
We are using a fork of Mongoose pending the merging of this PR:
https://github.com/cesanta/mongoose/pull/2301
The embedded web server will make our HTTP-based tests independent of
httpbin.org, which will make them quick and reliable.
Rebasing #1155 and applying some fixes. Had to comment out the
`mpd_notify_muxer_listener_unittest` because it depends on
`MockMpdNotifier` from `mpd/base` which has not been ported yet. Can
bring this test back once that has been ported.
Related to #1047
---------
Co-authored-by: Carlos Bentzen <carlos.bentzen@bitmovin.com>
Co-authored-by: Joey Parrish <joeyparrish@google.com>
This reverts commit 2e349845c6 from PR
#1203.
We are moving away from httpbin.org for testing, and this revert will
make subsequent changes to the tests easier to read.
Closing the upstream on flush will effectively terminate the ongoing
curl connection. This means that we would need re-establish the
connection in order to resume writing, this is not what we want. In the
spirit of the documentation of File::Flush
```c++
/// Flush the file so that recently written data will survive an
/// application crash (but not necessarily an OS crash). For
/// instance, in LocalFile the data is flushed into the OS but not
/// necessarily to disk.
```
We will instead wait for the curl thread to finish consuming what ever
might be in the upload cache, but leave the connection open for
subsequent writes.
Fixes#1196
# Low Latency DASH - `availabilityTimeComplete=false`
Low Latency DASH manifests generated by Packager were missing the
attribute `availabilityTimeComplete`. As per the [DASH
specs](https://dashif.org/docs/CR-Low-Latency-Live-r8.pdf):
**_the AdaptationSet@availabilityTimeCompleteshould be present and be
set to 'FALSE'_**
## The Issue
The missing attribute caused ULL streams from Shaka Packager to no
longer be compatible with DASH.js. Previous versions of DASH.js allowed
users to specify ULL mode when initializing the player. However, the
most recent releases of DASH.js automatically detect ULL by scanning the
manifest for ULL specific attributes. Although there are many attributes
only associated with ULL, [DASH.js only greps for
`availabilityTimeComplete` in its detection
logic](https://github.com/Dash-Industry-Forum/dash.js/blob/development/src/streaming/controllers/PlaybackController.js#L792-L805).
Because of the missing attribute in Packager and the limited ULL
verification criteria by DASH.js, Packager streams were not being
treated as low latency streams by DASH.js.
## Testing
### Unit Testing
`./mpd_unittest
--gtest_filter="SegmentTemplateTest.OneSegmentLowLatency"`
` ./mpd_unittest
--gtest_filter="LowLatencySegmentTest.LowLatencySegmentTemplate"`
### Manual Testing
- Created a low latency stream with Shaka Packager
- Observed the expected `availabilityTimeComplete=false` attribute in
the generated DASH manifest.
It appears that not all Apple implementations follow the HLS guidelines.
While the DEFAULT=NO for an audio track should be optional and default
to NO, in practice native HLS players Safari and iOS devices treat the
missing DEFAULT as a MAYBE.
Fixes#1169
Using a rolling release of OpenSUSE was unstable. But there is another
way around the old compiler in OpenSUSE 15. This upgrades the compiler
while staying on a stable release of the OS.
A single-line change on #L170 to `wv.protection_scheme =
struct.unpack('>L', bytes(protection_scheme, encoding='utf-8'))[0]`,
needed to work around this issue on Ubuntu 22.04LTS+ running Python
3.10+:
```sh
TypeError: a bytes-like object is required, not 'str'
```
On line 170.
This issue is observed on Python 3.10+ and above.
This workaround addresses a major backwards compatibility break with a
major Python release version where collections.abc isn't available.
Fixes#1192
A single-line change on #L170 to `wv.protection_scheme =
struct.unpack('>L', bytes(protection_scheme, encoding='utf-8'))[0]`,
needed to work around this issue on Ubuntu 22.04LTS+ running Python
3.10+:
```sh
TypeError: a bytes-like object is required, not 'str'
```
On line 170.
This removes all chromium dependencies from media/base/ and completes
the build system in CMake.
The ClosureThread class and its classes were removed, as they were
specific to chromium base. ClosureThread has been replaced by
std::thread.
The byte-swapping utilities in network_util.cc have been removed and
replaced with absl.
generate_unique_temp_path() was split out of file_unittest.cc into
file_test_util.cc, where other test suites could make use of it.
WARN_UNUSED_RESULT was replaced with the C++ standard attribute
[[nodiscard]].
The base::Clock interface was replaced with a typedef for a function
pointer that returns the current time.
This re-enables the tests in http_key_fetcher_unittest.cc by using
httpbin.org.
Issue #1047 (CMake porting)
Issue #346 (absl porting)
This does not depend on absl, which frees macros.h from any library deps
that would make the CMake dependency tree more complicated. This also
fixes build errors in some environments.
mbedtls works very differently from BoringSSL, and many changes had to
be made in the details of AES decryption to accomodate this.
Beyond the basic changes required for mbedtls, part of the CTS padding
implementation had to be rewritten. I believe this is because of an
assumption that held for BoringSSL, but not for mbedtls. I was unable to
determine what it was, so I rewrote the CTS decryption using reference
materials. After this, tests passed.
The deterministc PRNG I used with mbedtls in the RSA tests differs
somewhat from the old one, so the expected vectors had to be
regenerated. The old determinstic tests were also disabled, and are now
re-enabled.
Since cryptography is sensitive code, and because there were far more
changes needed here than just updating some headers and utility function
calls, this has been split into its own PR for separate review from the
rest of the media/base porting work.
Issue #1047 (CMake porting)
Issue #346 (absl porting)
This brings some workflow improvements and fixes from the `cmake` branch
to `main`, as well as some unique fixes to keep gclient working, so that
we can continue to accept contributions in `main` until the `cmake`
merge is ready.
- Fix docs build in GitHub Actions (from `cmake` branch)
- Cancel workflow when a PR is updated (from `cmake` branch)
- Fix docker failures caused by running as root (from `cmake` branch)
- Work around exception in depot_tools on Windows
- Use Windows 2019 images in GitHub Actions for compatibility with gyp
- Remove Docker build on ArchLinux, which no longer supports python2 at
all
- (NOTE: The `cmake` branch is still building on ArchLinux. Docker
builds for Arch will be restored to the `main` branch when the `cmake`
branch is finally merged to `main`.)
This is an automated sync of common workflows for this organization.
The upstream source is:
8bfe75f0d2
Co-authored-by: Shaka Bot <shaka-bot@users.noreply.github.com>
Rewrite test_data_util.cc to locate files relative to the source file
itself, rather than using a service from chromium `base::`.
Issue #1047 (CMake porting)
Issue #346 (absl porting)
The hack of using "secrets" to store per-repo settings was not working.
The main reason is that pull_request workflows don't have access to
secrets no matter what you do. So it was impossible to make this work
for settings like "ENABLE_SELF_HOSTED" for PR tests.
This change replaces that old hack with a new one. Now a repo owner must
create a "GitHub Environment" with the name of the setting they want to
enable. Currently supported values are "self_hosted", to add self-hosted
runners to the build/test matrix, and "debug", to start an SSH server
for debugging when a workflow fails.
Issue #1047 (CMake porting)
Joey Parrish
Cosmin Stejerean
Joey Parrish
Carlos Bentzen
Peter Zebühr
Caitlin O'Callaghan
Dennis E. Mungai
Marcus Spangenberg
Mark Sargent
Bartek Zdanowski
Shaka Bot