Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add startwithSAP/subsegmentstartswithSAP for audio tracks #1346

Merged
merged 6 commits into from
Feb 23, 2024

Conversation

cosmin
Copy link
Contributor

@cosmin cosmin commented Feb 16, 2024

Replaces #1055

This pull request adds startwithSAP/subsegmentstartswithSAP for aac, ac3, ec3 and ac4 audio tracks according to LIVE or VOD profile.

Closes #364

@cosmin
Copy link
Contributor Author

cosmin commented Feb 21, 2024

Puzzled by how that test is failing only on Windows release static.

@joeyparrish
Copy link
Member

Puzzled by how that test is failing only on Windows release static.

I see an "actual" MPD containing: <!--Generated with https://github.com/shaka-project/shaka-packager version 00fc734-release-->, but that line is missing from "expected".

Perhaps the mechanism for stripping that is failing on a particular build.

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looking into the mystery test failure.

@@ -288,6 +288,13 @@ std::optional<xml::XmlNode> AdaptationSet::GetXml() {
return std::nullopt;
}
}
if (subsegment_start_with_sap_ == 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why store a uint8_t, then only use it if it's 1? Other values are defined in the spec, such as 2. Could this maybe set the integer attribute with any non-zero value?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change and updated the PR, and then approved it. If you agree with the change, feel free to merge. Otherwise, you can undo this part.

@joeyparrish
Copy link
Member

I'm not certain, but it looks like OnDemandMpdBuilderTest is missing a call to SetPackagerVersionForTesting(). This writes to a global, so the test order may matter. If an earlier test called it, then OnDemandMpdBuilderTest may be benefiting from the earlier value. And the test order may have to do with the order of how things are linked, which may explain how a different build configuration could impact this.

@joeyparrish
Copy link
Member

I'm going to test this theory about the failed test by running individual suites in separate invocations using a filter on the command-line. If I'm right, I should be able to trigger a failure on any platform and build config using a filter. This can also help hunt down other missing calls to SetPackagerVersionForTesting().

@joeyparrish
Copy link
Member

I was wrong. First, I overlooked this line: xml_compare.cc:128] Attributes of element AdaptationSet do not match.

Second, xml_compare.cc seems not to care about comments or about the order of attributes.

The actual difference is:

Expected:

...
<AdaptationSet contentType="video" maxWidth="720" maxHeight="480" maxFrameRate="10/1" id="0">
...

Actual:

...
<AdaptationSet contentType="video" maxWidth="720" maxHeight="480" subsegmentStartsWithSAP="1" maxFrameRate="10/1" id="0">
...

That was very hard to spot, but actual has subsegmentStartsWithSAP="1", and expected does not.

@joeyparrish
Copy link
Member

One of the failing tests is OnDemandMpdBuilderTest.TwoVideosWithDifferentResolutions, which compares output against kFileNameExpectedMpdOutputVideo1And2 which is video_media_info1and2_expected_mpd_output.txt. That file does not mention subsegmentStartsWithSAP... so how is the test passing?

I'll look into xml_compare more deeply.

@joeyparrish
Copy link
Member

I added some logs to the comparison and just ran the one test on Linux. When it passes, I see that the actual MPD is missing subsegmentStartsWithSAP:

<AdaptationSet contentType="video" maxWidth="720" maxHeight="480" maxFrameRate="10/1" id="0">

This leads me to think we have an uninitialized value involved.

@joeyparrish
Copy link
Member

packager/mpd/base/adaptation_set.h doesn't initialize the new fields you added. In a release build, where memory is generally not sanitized for you on allocation, they happen to have a starting value that is non-zero.

@joeyparrish
Copy link
Member

The test should be fixed now.

@cosmin cosmin merged commit d23cce8 into shaka-project:main Feb 23, 2024
32 checks passed
joeyparrish pushed a commit to joeyparrish/shaka-packager that referenced this pull request Mar 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.0.0](v3.0.2...v4.0.0)
(2024-03-12)


### ⚠ BREAKING CHANGES

* Update all dependencies
* Drop Python 2 support in all scripts
* Replace glog with absl::log, tweak log output and flags
* Replace gyp build system with CMake

### Features

* Add input support for EBU Teletext in MPEG-TS
([shaka-project#1344](https://github.com/joeyparrish/shaka-packager/issues/1344))
([71c175d](71c175d))
* Add install target to build system
([3e71302](3e71302))
* Add PlayReady support in HLS.
([shaka-project#1011](https://github.com/joeyparrish/shaka-packager/issues/1011))
([96efc5a](96efc5a))
* add startwithSAP/subsegmentstartswithSAP for audio tracks
([shaka-project#1346](https://github.com/joeyparrish/shaka-packager/issues/1346))
([d23cce8](d23cce8))
* Add support for ALAC codec
([shaka-project#1299](https://github.com/joeyparrish/shaka-packager/issues/1299))
([b68ec87](b68ec87))
* Add support for single file TS for HLS
([shaka-project#934](https://github.com/joeyparrish/shaka-packager/issues/934))
([4aa4b4b](4aa4b4b))
* Add support for the EXT-X-START tag
([shaka-project#973](https://github.com/joeyparrish/shaka-packager/issues/973))
([76eb2c1](76eb2c1))
* Add xHE-AAC support
([shaka-project#1092](https://github.com/joeyparrish/shaka-packager/issues/1092))
([5d998fc](5d998fc))
* Allow LIVE UDP WebVTT input
([shaka-project#1349](https://github.com/joeyparrish/shaka-packager/issues/1349))
([89376d3](89376d3))
* **DASH:** Add Label element.
([shaka-project#1175](https://github.com/joeyparrish/shaka-packager/issues/1175))
([b1c5a74](b1c5a74))
* **DASH:** Add video transfer characteristics.
([shaka-project#1210](https://github.com/joeyparrish/shaka-packager/issues/1210))
([8465f5f](8465f5f))
* default text zero bias
([shaka-project#1330](https://github.com/joeyparrish/shaka-packager/issues/1330))
([2ba67bc](2ba67bc))
* Drop Python 2 support in all scripts
([3e71302](3e71302))
* Generate the entire AV1 codec string when the colr atom is present
([shaka-project#1205](https://github.com/joeyparrish/shaka-packager/issues/1205))
([cc9a691](cc9a691)),
closes
[shaka-project#1007](https://github.com/joeyparrish/shaka-packager/issues/1007)
* HLS / DASH support forced subtitle
([shaka-project#1020](https://github.com/joeyparrish/shaka-packager/issues/1020))
([f73ad0d](f73ad0d))
* Move all third-party deps into git submodules
([shaka-project#1083](https://github.com/joeyparrish/shaka-packager/issues/1083))
([3e71302](3e71302))
* order streams in manifest based on command-line order
([shaka-project#1329](https://github.com/joeyparrish/shaka-packager/issues/1329))
([aad2a12](aad2a12))
* Parse MPEG-TS PMT ES language and maximum bitrate descriptors
([shaka-project#369](https://github.com/joeyparrish/shaka-packager/issues/369))
([shaka-project#1311](https://github.com/joeyparrish/shaka-packager/issues/1311))
([c09eb83](c09eb83))
* Portable, fully-static release executables on Linux
([shaka-project#1351](https://github.com/joeyparrish/shaka-packager/issues/1351))
([9be7c2b](9be7c2b))
* Replace glog with absl::log, tweak log output and flags
([3e71302](3e71302))
* Replace gyp build system with CMake
([3e71302](3e71302)),
closes
[shaka-project#1047](https://github.com/joeyparrish/shaka-packager/issues/1047)
* Respect the file mode for HttpFiles
([shaka-project#1081](https://github.com/joeyparrish/shaka-packager/issues/1081))
([3e71302](3e71302))
* This patch adds support for DTS:X Profile 2 audio in MP4 files.
([shaka-project#1303](https://github.com/joeyparrish/shaka-packager/issues/1303))
([07f780d](07f780d))
* Update all dependencies
([3e71302](3e71302))
* Write colr atom to muxed mp4
([shaka-project#1261](https://github.com/joeyparrish/shaka-packager/issues/1261))
([f264bef](f264bef)),
closes
[shaka-project#1202](https://github.com/joeyparrish/shaka-packager/issues/1202)


### Bug Fixes

* Accept 100% when parsing WEBVTT regions
([shaka-project#1006](https://github.com/joeyparrish/shaka-packager/issues/1006))
([e1b0c7c](e1b0c7c)),
closes
[shaka-project#1004](https://github.com/joeyparrish/shaka-packager/issues/1004)
* Add missing &lt;cstdint&gt; includes
([shaka-project#1306](https://github.com/joeyparrish/shaka-packager/issues/1306))
([ba5c771](ba5c771)),
closes
[shaka-project#1305](https://github.com/joeyparrish/shaka-packager/issues/1305)
* Add missing limits header
([efbca39](efbca39))
* Always log to stderr by default
([shaka-project#1350](https://github.com/joeyparrish/shaka-packager/issues/1350))
([35c2f46](35c2f46)),
closes
[shaka-project#1325](https://github.com/joeyparrish/shaka-packager/issues/1325)
* AudioSampleEntry size caluations due to bad merge
([shaka-project#1354](https://github.com/joeyparrish/shaka-packager/issues/1354))
([615720e](615720e))
* **CI:** Add Mac-arm64 to build matrix
([shaka-project#1359](https://github.com/joeyparrish/shaka-packager/issues/1359))
([c456ad6](c456ad6))
* **CI:** Add missing Linux arm64 builds to release
([9c033b9](9c033b9))
* dash_roles add role=description for DVS audio per DASH-IF-IOP-v4.3
([shaka-project#1054](https://github.com/joeyparrish/shaka-packager/issues/1054))
([dc03952](dc03952))
* Don't close upstream on HttpFile::Flush
([shaka-project#1201](https://github.com/joeyparrish/shaka-packager/issues/1201))
([53d91cd](53d91cd)),
closes
[shaka-project#1196](https://github.com/joeyparrish/shaka-packager/issues/1196)
* duplicate representation id for TTML when forced ordering is on
([shaka-project#1364](https://github.com/joeyparrish/shaka-packager/issues/1364))
([0fd815a](0fd815a)),
closes
[shaka-project#1362](https://github.com/joeyparrish/shaka-packager/issues/1362)
* duration formatting and update mpd testdata to reflect new format
([shaka-project#1320](https://github.com/joeyparrish/shaka-packager/issues/1320))
([56bd823](56bd823))
* Explicitly signal the lack of CEA captions in HLS
([d48bf0f](d48bf0f)),
closes [shaka-project#922](https://github.com/joeyparrish/shaka-packager/issues/922)
* Fix build errors related to std::numeric_limits
([shaka-project#972](https://github.com/joeyparrish/shaka-packager/issues/972))
([9996c73](9996c73))
* Fix build on FreeBSD
([shaka-project#1287](https://github.com/joeyparrish/shaka-packager/issues/1287))
([3e71302](3e71302))
* Fix clang build
([shaka-project#1288](https://github.com/joeyparrish/shaka-packager/issues/1288))
([3e71302](3e71302))
* Fix crash in static-linked linux builds
([e2d66b3](e2d66b3)),
closes [shaka-project#996](https://github.com/joeyparrish/shaka-packager/issues/996)
* Fix failure fetching encryption keys
([7392d80](7392d80))
* Fix failure on very short WebVTT files
([shaka-project#1216](https://github.com/joeyparrish/shaka-packager/issues/1216))
([dab165d](dab165d)),
closes
[shaka-project#1217](https://github.com/joeyparrish/shaka-packager/issues/1217)
* Fix handling of non-interleaved multi track FMP4 files
([shaka-project#1214](https://github.com/joeyparrish/shaka-packager/issues/1214))
([dcf3225](dcf3225)),
closes
[shaka-project#1213](https://github.com/joeyparrish/shaka-packager/issues/1213)
* Fix issues with `collections.abc` in Python 3.10+
([shaka-project#1188](https://github.com/joeyparrish/shaka-packager/issues/1188))
([80e0240](80e0240)),
closes
[shaka-project#1192](https://github.com/joeyparrish/shaka-packager/issues/1192)
* Fix local files with UTF8 names
([shaka-project#1246](https://github.com/joeyparrish/shaka-packager/issues/1246))
([3e71302](3e71302))
* Fix missing newline at the end of usage
([shaka-project#1352](https://github.com/joeyparrish/shaka-packager/issues/1352))
([6276584](6276584))
* Fix Python 3.10+ compatibility in scripts
([3e71302](3e71302))
* Fix tags in official Docker images and binaries
([73a85ce](73a85ce)),
closes
[shaka-project#1366](https://github.com/joeyparrish/shaka-packager/issues/1366)
* Fix uninitialized value found by Valgrind
([shaka-project#1336](https://github.com/joeyparrish/shaka-packager/issues/1336))
([7ef5167](7ef5167))
* Fix various build issues on macOS
([3e71302](3e71302))
* Fix various build issues on Windows
([3e71302](3e71302))
* hls, set the DEFAULT explicitly to NO. Supports native HLS players.
([shaka-project#1170](https://github.com/joeyparrish/shaka-packager/issues/1170))
([1ab6818](1ab6818)),
closes
[shaka-project#1169](https://github.com/joeyparrish/shaka-packager/issues/1169)
* http_file: Close upload cache on task exit
([shaka-project#1348](https://github.com/joeyparrish/shaka-packager/issues/1348))
([6acdcc3](6acdcc3)),
closes
[shaka-project#1347](https://github.com/joeyparrish/shaka-packager/issues/1347)
* Indexing `bytes` produces `int` on python3 for `pssh-box.py`
([shaka-project#1228](https://github.com/joeyparrish/shaka-packager/issues/1228))
([d9d3c7f](d9d3c7f)),
closes
[shaka-project#1227](https://github.com/joeyparrish/shaka-packager/issues/1227)
* Low Latency DASH: include the "availabilityTimeComplete=false"
attribute
([shaka-project#1198](https://github.com/joeyparrish/shaka-packager/issues/1198))
([d687ad1](d687ad1))
* misleading log output when HLS target duration updates (fixes
[shaka-project#969](https://github.com/joeyparrish/shaka-packager/issues/969))
([shaka-project#971](https://github.com/joeyparrish/shaka-packager/issues/971))
([f7b3986](f7b3986))
* **MP4:** Add compatible brand dby1 for Dolby content.
([shaka-project#1211](https://github.com/joeyparrish/shaka-packager/issues/1211))
([520926c](520926c))
* Parse one frame mpeg-ts video
([shaka-project#1015](https://github.com/joeyparrish/shaka-packager/issues/1015))
([b221aa9](b221aa9)),
closes
[shaka-project#1013](https://github.com/joeyparrish/shaka-packager/issues/1013)
* preserve case for stream descriptors
([shaka-project#1321](https://github.com/joeyparrish/shaka-packager/issues/1321))
([5d44368](5d44368))
* Prevent crash in GetEarliestTimestamp() if periods are empty
([shaka-project#1173](https://github.com/joeyparrish/shaka-packager/issues/1173))
([d6f28d4](d6f28d4)),
closes
[shaka-project#1172](https://github.com/joeyparrish/shaka-packager/issues/1172)
* PTS diverge DTS when DTS close to 2pow33 and PTS more than 0
([shaka-project#1050](https://github.com/joeyparrish/shaka-packager/issues/1050))
([ab8ab12](ab8ab12)),
closes
[shaka-project#1049](https://github.com/joeyparrish/shaka-packager/issues/1049)
* remove extra block assumptions in mbedtls integration
([shaka-project#1323](https://github.com/joeyparrish/shaka-packager/issues/1323))
([db59ad5](db59ad5)),
closes
[shaka-project#1316](https://github.com/joeyparrish/shaka-packager/issues/1316)
* Restore support for legacy FairPlay system ID
([shaka-project#1357](https://github.com/joeyparrish/shaka-packager/issues/1357))
([4d22e99](4d22e99))
* Roll back depot_tools, bypass vpython
([shaka-project#1045](https://github.com/joeyparrish/shaka-packager/issues/1045))
([3fd538a](3fd538a)),
closes
[shaka-project#1023](https://github.com/joeyparrish/shaka-packager/issues/1023)
* set array_completeness in HEVCDecoderConfigurationRecord correctly
([shaka-project#975](https://github.com/joeyparrish/shaka-packager/issues/975))
([270888a](270888a))
* TTML generator timestamp millisecond formatting
([shaka-project#1179](https://github.com/joeyparrish/shaka-packager/issues/1179))
([494769c](494769c)),
closes
[shaka-project#1180](https://github.com/joeyparrish/shaka-packager/issues/1180)
* Update golden files for ttml tests and failing hls unit tests.
([shaka-project#1226](https://github.com/joeyparrish/shaka-packager/issues/1226))
([ac47e52](ac47e52))
* Update to use official FairPlay UUID.
([shaka-project#1281](https://github.com/joeyparrish/shaka-packager/issues/1281))
([ac59b9e](ac59b9e))
* use a better estimate of frame rate for cases with very short first
sample durations
([shaka-project#838](https://github.com/joeyparrish/shaka-packager/issues/838))
([5644041](5644041))
* webvtt single cue do not fail on EOS
([shaka-project#1061](https://github.com/joeyparrish/shaka-packager/issues/1061))
([b9d477b](b9d477b)),
closes
[shaka-project#1018](https://github.com/joeyparrish/shaka-packager/issues/1018)

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Apr 24, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

startWithSAP & subsegmentStartsWithSAP attributes are not populated and this is not spec compliant
3 participants