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.

Sign up for GitHub

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

Jump to bottom

fix: Fix support for scte214:supplementalCodecs and SUPPLEMENTAL-CODECS #7456

Closed
wants to merge 3 commits into from
Closed

fix: Fix support for scte214:supplementalCodecs and SUPPLEMENTAL-CODECS #7456

wants to merge 3 commits into from

Conversation

gmcgarry
Copy link
Contributor

@gmcgarry gmcgarry commented Oct 21, 2024 •
edited
Loading

Currently the codec adds supplemental codecs to the list of allCodecs and has an implicit priority order to select a codec. Support for the codec is tested with MediaSource.isTypeSupported().

There are multiple issues with this approach:

  • the priority is implicit
  • MediaSource.isTypeSupported() decision is effectively overriding platform-specific MediaCapabilities logic
  • MediaSource.isTypeSupported() is frequently wrong depending on whether the content is encrypted

Speaking in DASH parlance, this patch considers that scte214:supplementalCodecs is effectively a short-hand mechanism to specify an alternate variant. In fact, shaka-packager implements the mechanism of supplementalCodecs by duplicating the AdaptationSet with a different @codecs attribute.

This patch duplicates the variant with each supplemental codec and defers codec priority and codec selection until after MediaCapabilities.decodingInfo().

This fixes support for DolbyVision Profile 8, which is otherwise broken because the latest browsers always return false from MediaSource.isTypeSupported(), and DolbyVision is commonly only available in a secure hardware decode pipeline.

@gmcgarry
Squashed commit of the following:
0ab211b 
commit 4cd76817a514f2e6fec4cf4adf37ca1f484569cb
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 15:21:32 2024 +1000

    Avoid infinite loop after fixing const assignment.

commit 532ac2d9d8857c80ca18e7ef2b26f14461d2197c
Merge: c5f144a86 6a519767d
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 14:24:35 2024 +1000

    Merge branch 'dolby/disable-supplemental-codecs' into dolby/fix-supplemental-codecs

commit 6a519767d96d9610c6d41b531352b75fdfb14705
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 14:23:20 2024 +1000

    Add missing comma.

commit c5f144a86abd24b40bb1b59d1c34c6f530739ed6
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 14:22:52 2024 +1000

    Add missing "include".

commit 1dfbbd5ace89c9542a99c0d3598dc395b9835c31
Merge: fcefce871 e3ee8a42d
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 14:15:51 2024 +1000

    Merge branch 'dolby/disable-supplemental-codecs' into dolby/fix-supplemental-codecs

    # Conflicts:
    #	lib/hls/hls_parser.js

commit fcefce871dd4231ffe5ee1c692334fa562b5b309
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 14:13:54 2024 +1000

    Fix formatting.

commit 3d5556a409b0f119c655950b96df13f3025a3eee
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 13:48:21 2024 +1000

    Fix supplementalCodecs for HLS too.

commit e3ee8a42d1eec45ae4250fc45d609da3497cc35f
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 12:11:41 2024 +1000

    Add ignoreSupplementalCodecs into the right place for HLS.

commit 65344bc69426d8d46b33a445acac300da7b5ea68
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 09:55:16 2024 +1000

    Also ignore SUPPLEMENTAL-CODECS attribute in HLS manifest.

commit 84766636e65f0741acf617eabf9942b41b7f35fb
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 09:51:12 2024 +1000

    Revert "Fix unused include."

    This reverts commit 682a0ddb1b6fef75c0328858b5dea479c0f75b0c.

commit 6df27c773adcc8b73b01cba86793bcd4bdd656cc
Author: Gregory McGarry <[email protected]>
Date:   Wed Aug 7 09:50:53 2024 +1000

    Revert "Remove chooseBetterCodecs(), since it doesn't work properly for encrypted content on Win11."

    This reverts commit d44a067cf173e6e99527e6b61fa9a0ddfe99a8a4.

commit f7847af58eb7674b17632eeedf609dd8373cc8bf
Author: Gregory McGarry <[email protected]>
Date:   Tue Aug 6 15:38:42 2024 +1000

    Remove unused constant.

commit b9e71e5e635338811d4e8d9fd5abe36f50191d22
Author: Gregory McGarry <[email protected]>
Date:   Tue Aug 6 15:37:44 2024 +1000

    Nuke old supplementalProperties.

commit 682a0ddb1b6fef75c0328858b5dea479c0f75b0c
Author: Gregory McGarry <[email protected]>
Date:   Tue Aug 6 15:37:25 2024 +1000

    Fix unused include.

commit 7069f03b678524e88d52778f95a846936012b846
Author: Gregory McGarry <[email protected]>
Date:   Tue Aug 6 15:37:07 2024 +1000

    Fix formatting.

commit d44a067cf173e6e99527e6b61fa9a0ddfe99a8a4
Author: Gregory McGarry <[email protected]>
Date:   Tue Aug 6 14:55:45 2024 +1000

    Remove chooseBetterCodecs(), since it doesn't work properly for encrypted content on Win11.

commit b4f1598ce5749f7f9492220301f8bcdc410eae3a
Author: Gregory McGarry <[email protected]>
Date:   Tue Aug 6 14:26:20 2024 +1000

    Duplicate representation to handle supplementalCodecs.  This ensure that MediaCapabilities API is used for stream selection rather than using MediaSource::isTypeSupported() which doesn't work correctly for Dolby Vision.

commit a432bd650e25436915846403dda9177a3ef88378
Merge: ca8c18277 5a4c9af63
Author: Gregory McGarry <[email protected]>
Date:   Fri Jul 19 14:52:39 2024 +1000

    Merge branch 'dolby/disable-supplemental-codecs' into dolby/gmcga-build

commit ca8c18277e0ceb659c1cfedaca8bdd0344b789ea
Merge: 241b84304 bf072fb
Author: Gregory McGarry <[email protected]>
Date:   Fri Jul 19 14:52:17 2024 +1000

    Merge remote-tracking branch 'origin/dolby/fairplay2' into dolby/gmcga-build

commit 5a4c9af6320007f0e293ae68fcee4ebb445bba3f
Author: Gregory McGarry <[email protected]>
Date:   Fri Jul 19 14:51:01 2024 +1000

    Provide configuration switch to disable supplementalCodecs.

commit 241b8430453670766d027ff7672c1d68307bdbd8
Merge: f161a1c 8f96e72
Author: Gregory McGarry <[email protected]>
Date:   Fri Jul 19 14:48:41 2024 +1000

    Merge branch 'dolby/fix-rejectCodec2' into dolby/gmcga-build

commit 8f96e72
Author: Gregory McGarry <[email protected]>
Date:   Fri Jul 19 14:25:30 2024 +1000

    Fix rejectCodec_() on Safari which uses ManagedMediaSource.

commit f5c6ad8
Author: Gregory McGarry <[email protected]>
Date:   Thu Jul 18 13:57:33 2024 +1000

    Correctly get the base codec from a mime-type.

commit bf072fb
Author: Gregory McGarry <[email protected]>
Date:   Mon May 6 11:19:16 2024 +1000

    Enable FairPlay.
@gmcgarry
Remove future code to ignore supplementalCodecs.
1d58cb0
@gmcgarry
Add missing commit.
013b8b3
@avelad avelad added type: bug Something isn't working correctly component: HLS The issue involves Apple's HLS manifest format priority: P2 Smaller impact or easy workaround component: DASH The issue involves the MPEG DASH manifest format labels Oct 21, 2024
@avelad avelad added this to the v4.12 milestone Oct 21, 2024
@shaka-bot
Copy link
Collaborator

Incremental code coverage: 100.00%

avelad
avelad requested changes Oct 23, 2024
View reviewed changes
Copy link
Member

@avelad avelad left a comment

Choose a reason for hiding this comment

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

Please, add some tests for it.

@avelad avelad added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Oct 23, 2024
@shaka-bot
Copy link
Collaborator

Closing due to inactivity. If the author would like to continue this PR, they can reopen it (preferred) or start a new one (if needed).

@shaka-bot shaka-bot closed this Oct 30, 2024
@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Oct 30, 2024
@gmcgarry
Copy link
Contributor Author

Added some missing tests as part of PR7455. Can expand. Can this PR be re-opened to update PR?

@avelad avelad reopened this Nov 20, 2024
@avelad
Copy link
Member

avelad commented Nov 27, 2024

Added some missing tests as part of PR7455. Can expand. Can this PR be re-opened to update PR?

Any update?

@avelad avelad modified the milestones: v4.12, v4.13 Dec 5, 2024
@avelad
Copy link
Member

avelad commented Dec 5, 2024

I'm going to create a separate PR with all the necessary fixes, it seems that when you created the PR you didn't check the option to allow maintainers to edit your PR :(

@avelad
Copy link
Member

avelad commented Dec 5, 2024

#7720

@avelad avelad closed this Dec 5, 2024
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Feb 3, 2025
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Feb 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@avelad avelad avelad requested changes

@joeyparrish joeyparrish Awaiting requested review from joeyparrish

@theodab theodab Awaiting requested review from theodab

@tykus160 tykus160 Awaiting requested review from tykus160

Assignees
No one assigned
Labels
component: DASH The issue involves the MPEG DASH manifest format component: HLS The issue involves Apple's HLS manifest format priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Milestone
v4.13
Development

Successfully merging this pull request may close these issues.
3 participants
@gmcgarry @shaka-bot @avelad