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: HLS / DASH support forced subtitle #1020

Merged
merged 24 commits into from
Feb 15, 2024

Conversation

vish91
Copy link
Contributor

@vish91 vish91 commented Jan 14, 2022

Closes #988

@joeyparrish
Copy link
Member

If I understand this correctly, to get forced-subs and output both DASH & HLS, one would have to indicate a dash role for forced subs and set the forced attribute to get the same effect in HLS?

@vish91
Copy link
Contributor Author

vish91 commented Apr 28, 2022

If I understand this correctly, to get forced-subs and output both DASH & HLS, one would have to indicate a dash role for forced subs and set the forced attribute to get the same effect in HLS?

@joeyparrish yeah I sync'd this branch with master to re-run actions and see if we can get some logs from the builds.
meanwhile some background on this is in here: #988
Yeah dash does need support for a new role for this

@vish91 vish91 changed the title HLS / DASH support forced subtitle add: HLS / DASH support forced subtitle Apr 28, 2022
@vish91 vish91 changed the title add: HLS / DASH support forced subtitle feat: HLS / DASH support forced subtitle Apr 28, 2022
@joeyparrish
Copy link
Member

This seems fine, but I would strongly prefer it (from a usability standpoint) if forced=1 implied adding forced-subtitle to dash_roles. I don't think you should have to use both if you're already setting forced=1 for HLS.

@vish91
Copy link
Contributor Author

vish91 commented Jun 2, 2022

This seems fine, but I would strongly prefer it (from a usability standpoint) if forced=1 implied adding forced-subtitle to dash_roles. I don't think you should have to use both if you're already setting forced=1 for HLS.

@joeyparrish yeah we discussed that here #988 . The problem is dash has roles field to signal vs HLS doesn't have anything like that so far. Hence the differentiation between how to trigger these for HLS vs DASH.

@joeyparrish
Copy link
Member

The problem is dash has roles field to signal vs HLS doesn't have anything like that so far. Hence the differentiation between how to trigger these for HLS vs DASH.

I understand, but why couldn't you check for the HLS field in the DASH code, and insert the role automatically?

@vish91
Copy link
Contributor Author

vish91 commented Jun 2, 2022

The problem is dash has roles field to signal vs HLS doesn't have anything like that so far. Hence the differentiation between how to trigger these for HLS vs DASH.

I understand, but why couldn't you check for the HLS field in the DASH code, and insert the role automatically?

yeah we could technically. Was discussing it with KQ initially, but that drifts us away for this being a DASH role from the spec prespective and users having to change their command to support a new parameter instead of actually using the dash_role param for setting the role to forced-subtitle.

@joeyparrish
Copy link
Member

I'm not suggesting you drop the dash_role part. But if you're generating both HLS & DASH in one command, why not say that forced=1 implies the appropriate role on the DASH side?

@vish91
Copy link
Contributor Author

vish91 commented Jun 3, 2022

I'm not suggesting you drop the dash_role part. But if you're generating both HLS & DASH in one command, why not say that forced=1 implies the appropriate role on the DASH side?

hmm.. would you mind sharing an example for this.. I wanna see it and test it before I can say we can change it or not. This is an input paramter and not packager parameter so thinking out loud here
would you want to allow a user to pass either dash_role or forced on the input parameter and be able to support this feature ?

@vish91
Copy link
Contributor Author

vish91 commented Jun 15, 2022

@joeyparrish any examples or sample command on what you were explaining ?

@vish91
Copy link
Contributor Author

vish91 commented Jan 3, 2024

@joeyparrish if you wanna take a pass at this PR , I updated it with the main branch so all the changes are in here

@cosmin
Copy link
Contributor

cosmin commented Feb 14, 2024

I think we should call the stream descriptor forced_subtitle and have it be generic for both dash and hls, if set it would apply the dash role (in addition to being able to directly set the dash_roles manually), set FORCED=YES for HLS, and validate it's only supplied on subtitle streams.

And remove all the various hls only referendes in the code.

@vish91
Copy link
Contributor Author

vish91 commented Feb 14, 2024

@cosmin @joeyparrish yeah sure i am ok with that, however in that case what happens when someone doesn't set stream_descriptor and has just set the DASH role value in stream_descriptor ? Will it fail silently or error ?
So for packager behavior will we ask users to ensure that they set dash_role=forced-subtitle and forced_subtitle=1 to achieve success for DASH and for HLS they only have to set forced_subtitle=1.

If that would be the expectation we can go that route. I might not have enough time to rework this PR by tonight. Might need some time to look into DASH flow for that.

@joeyparrish
Copy link
Member

@cosmin wrote:

I think we should call the stream descriptor forced_subtitle and have it be generic for both dash and hls, if set it would apply the dash role (in addition to being able to directly set the dash_roles manually), set FORCED=YES for HLS, and validate it's only supplied on subtitle streams.

@vish91 wrote:

So for packager behavior will we ask users to ensure that they set dash_role=forced-subtitle and forced_subtitle=1 to achieve success for DASH and for HLS they only have to set forced_subtitle=1.

I think what Cosmin is saying is that forced_subtitle=1 in the stream descriptor would imply the appropriate DASH role. If no dash_role= is given, forced_subtitle=1 would set dash_role=forced-subtitle internally. dash_role is low-level, forced_subtitle is high-level. For those who just want forced subs, the high level flag should be enough. @cosmin, am I interpreting you correctly?

@cosmin
Copy link
Contributor

cosmin commented Feb 14, 2024

@joeyparrish that's correct

@vish91 I can make the updates if you're ok with it or can wait if you prefer to do it.

@vish91
Copy link
Contributor Author

vish91 commented Feb 14, 2024

yep go for it please. 🙇🏽‍♂️ Just one note if you do can you please adjust the documentation as well. I think we missed changing the force_cl_index as well so was meaning to update that as well in here.

@cosmin
Copy link
Contributor

cosmin commented Feb 15, 2024

Made the updates to do this as we discussed with forced_subtitle stream descriptor being the high level way of doing this, and having it set role internally for dash if needed.

Yup, that's a bear, eh.

00:00:01.000 --> 00:00:04.700 align:center
He 's... um... doing bear-like stuff.
Copy link
Member

Choose a reason for hiding this comment

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

🤣

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.

Looks great otherwise

packager/hls/base/media_playlist.cc Outdated Show resolved Hide resolved
@joeyparrish joeyparrish merged commit f73ad0d into shaka-project:main Feb 15, 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 <cstdint> 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 15, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 15, 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.

HLS / DASH support for forced subtitles
3 participants