[Proposal] scte214 - use supplemental codec instead of base codec if it's supported #1307

Florent-Bouisset · 2023-10-27T16:00:59Z

Context

This PR add partial support for the SCTE-214 related to Dash.
This PR add in particular the support of the scte214:supplementalCodecs attribute in .mpd Manifests

Supplemental codecs are defined as backwards-compatible codecs enhancing the experience of a base layer codec.
If the supplemental codecs are supported by the device, we should use them instead of the base codec, this would allow to play the medias with a better experience for users.

SCTE-214: https://www.scte.org/standards/library/catalog/scte-214-1-mpeg-dash-for-ip-based-cable-services-part1-mpd-constraints-and-extensions/

Implementation

The proposed logic for this improvement is:

On a representation, check if there is a supplementalCodec.
If there is none, use the base codec for the sourceBuffer and abort all remaining steps.
If there is one, use MediaSource.isTypeSupported on the supplementalCodec to verify if it's supported.
If the supplementalCodec is not supported, use the base codec for the sourceBuffer and abort all remaining steps.
If the supplementalCodec is supported, use the supplemental for the sourceBuffer.

Example

Given the following Manifest:

<Representation id="hvc1_850800=8" bandwidth="850800" width="640" height="360" sar="1:1"
        frameRate="25" codecs="hvc1.2.4.L63.B0" scte214:supplementalCodecs="dvh1.08.01"
        scte214:supplementalProfiles="db1p" segmentProfiles="chd1" />

On a device that does not support the codecs dvh1.08.01 the used codec should be hvc1.2.4.L63.B0
On a device that supports the codecs dvh1.08.01 the used codec should be dvh1.08.01

Work in progress

What about the supplementalProfiles, do we need to have the same logic ?
Should also parse the supplementalCodecs on the AdaptationSet
Documentation

peaBerberian

I also have two other questions:

Can supplementalCodec be defined at the AdaptationSet level in an MPD (like codec I think)? In which case we should also parse it when found there.
The WASM DASH parser also has to be updated, do you know how to do it or do you want me to do it instead?

src/manifest/representation.ts

test/suppCodec.mpd

Florent-Bouisset · 2023-11-07T15:35:14Z

I also have two other questions:

Can supplementalCodec be defined at the AdaptationSet level in an MPD (like codec I think)? In which case we should also parse it when found there.

The WASM DASH parser also has to be updated, do you know how to do it or do you want me to do it instead?

Yes the supplementalCodec can be defined at the AdaptationSet level.
There is example of it in the SCTE 214, so we should parse it.

peaBerberian · 2023-11-07T17:13:24Z

I also found out that both SCTE 214 and HLS* indicate the possibility of having multiple supplemental codecs, with a given separator (I think space for DASH SCTE214, and a comma for HLS).
I'm guessing we will repeat here spaces if we have them, so we may want to check what would be the isTypeSupported / addSourceBuffer behavior when encountering a space in the codecs part of the given type. This character may need to be replaced.

* note: we do not offer first class support for HLS yet - putting directfile mode on Safari aside, yet we still consider it as a possible future feature when architecturing things as long as it doesn't sacrifice too much maintainability, in case it become an important subject for us

…pported

myoann · 2023-11-09T10:09:51Z

src/parsers/manifest/dash/common/parse_representations.ts

@@ -213,6 +213,17 @@ export default function parseRepresentations(
      codecs = codecs === "mp4a.40.02" ? "mp4a.40.2" : codecs;
      parsedRepresentation.codecs = codecs;
    }
+
+    let supplementalCodecs: string | undefined;
+    if (representation.attributes.supplementalCodecs != null) {


Why don't you use a !==, in order to verify also the type or the existing isNullOrUndefined method in the utils?

It's used to check if the value is either null or undefined,
@peaBerberian is there any recommandations about using != or the function !isNullOrUndefined ?
Both syntax exists in the project

Yes we now try to avoid relying on double equals in the RxPlayer as:

double equals' behavior might not be understood by every dev

it's more explicit when reading to have the exact conditions written

it avoid some classes of bug where we wrote double equals just by lazyness.
Here the intent is to have to check explicitely for what can happen. In some situations where only null xor undefined are possible at first, we may create a bug in future code if ever the other value become possible.

There are some areas of the code still relying on the == null trick, but that's old logic we ultimately want to replace them by more explicit code.

Ok, should I change all the != to !isNullOrUndefined in the parse_representation.ts file, or should I do another MR for this change ?

As you want, both are good to me

Here isn't it only undefined though? I'm unsure

Yes I think too it's only undefined but as a precaution I didn't want to change the behavior of the API.

src/parsers/manifest/dash/js-parser/node_parsers/utils.ts

src/parsers/manifest/dash/common/parse_representations.ts

src/parsers/manifest/dash/js-parser/node_parsers/__tests__/utils.test.ts

src/manifest/representation.ts

peaBerberian · 2023-11-09T17:00:09Z

src/parsers/manifest/dash/common/parse_representations.ts

@@ -120,19 +122,19 @@ export default function parseRepresentations(
  const parsedRepresentations : IParsedRepresentation[] = [];
  for (const representation of representationsIR) {
    // Compute Representation ID
-    let representationID = representation.attributes.id != null ?
+    let representationID = !isNullOrUndefined(representation.attributes.id) ?


I'm under the impression than none of those attributes can be set to null, in which case the proper way of fixing that double equal to make it more explicit would be to just check against undefined instead

peaBerberian · 2023-11-09T17:04:37Z

I've been wondering.

Do we ever want to rely on the old codec value here if the supplementalCodec is used instead?
If yes, for what reason, to add more precision?

The only case we've now seen of actual supplementalCodec usage is for Dolby Vision, where the supplementalCodec seems to always be sufficient in itself, yet it's true we don't know what will come tomorrow.

In the scenario where we never want to rely on the non-used codec anyway, because it is already known which one we use on Representation instanciation, wouldn't it be less error-prone to just set codec to the actually used codec, without considering SCTE214 anymore at this level?

src/parsers/manifest/dash/js-parser/node_parsers/utils.ts

Florent-Bouisset · 2023-11-10T09:13:31Z

I've been wondering.

Do we ever want to rely on the old codec value here if the supplementalCodec is used instead? If yes, for what reason, to add more precision?

The only case we've now seen of actual supplementalCodec usage is for Dolby Vision, where the supplementalCodec seems to always be sufficient in itself, yet it's true we don't know what will come tomorrow.

In the scenario where we never want to rely on the non-used codec anyway, because it is already known which one we use on Representation instanciation, wouldn't it be less error-prone to just set codec to the actually used codec, without considering SCTE214 anymore at this level?

I think we can set codec with the used codec. It won't change the behavior and it will be simpler for other parts of the code relying on codec

One thing that I was wondering is about changing the device:
If the user is playing a video with the base codec on his mobile phone, and then cast to a TV that is compatible with the supplemental codec (ex: dolby vision), will we re-instanciate the representations ? If we don't, isSupplementalCodecSupported will still be set to false and we will be playing the base codec rather that the supplementalCodec on the TV that was compatible with both.

peaBerberian · 2023-11-10T09:42:09Z

If the user is playing a video with the base codec on his mobile phone, and then cast to a TV that is compatible with the supplemental codec (ex: dolby vision), will we re-instanciate the representations ?

All those kinds of usages I know of for now work with two pages, one on the sending device, and one on the receiving device, which both would run their RxPlayer locally.

Though I guess sending chunks from a sender to be decoded in a receiver should theoretically be possible, I'm unsure of how it would interact with the blocking MediaSource.isTypeSupported API. Though I don't remember if the new remote playback API will handle such cases.

…alCodec in the representation

peaBerberian · 2023-11-10T16:46:28Z

src/parsers/manifest/dash/common/indexes/utils.ts

@@ -30,3 +31,33 @@ export function getSegmentTimeRoundingError(timescale: number): number {
  return config.getCurrent().DEFAULT_MAXIMUM_TIME_ROUNDING_ERROR * timescale;
 }

+const supplementalCodecSeparator = /[, ]+/g;


Nice, just the indexes directory is normally only for what we call the segment "index" here, which the way segments are announced in the MPD.

So that's probably not the right place for this util

peaBerberian · 2023-11-10T16:49:08Z

LGTM, just I had one last comment about the file where one of the utils is declared

peaBerberian · 2023-11-13T09:47:44Z

I forgot can you unstage dist/mpd-parser.wasm ? We only update files in the dist directory at release time.

Florent-Bouisset added 2 commits October 27, 2023 15:32

parse supplemental codec

7d1ecef

add a property isSupplementalCoded supported and update getMimeType

85b5941

peaBerberian mentioned this pull request Nov 3, 2023

Check Dolby Atmos and Dolby Vision in current stream #1266

Closed

peaBerberian added this to the 3.33.0 milestone Nov 6, 2023

peaBerberian reviewed Nov 6, 2023

View reviewed changes

src/manifest/representation.ts Outdated Show resolved Hide resolved

src/manifest/representation.ts Outdated Show resolved Hide resolved

src/manifest/representation.ts Outdated Show resolved Hide resolved

test/suppCodec.mpd Outdated Show resolved Hide resolved

peaBerberian added the Priority: 1 (High) This issue or PR has a high priority. label Nov 7, 2023

add documentation for isSupplementalCodecSupported property

09e1d03

parse supplementalCodec in the adaptation set

6c67e53

Florent-Bouisset added 4 commits November 7, 2023 18:23

return the codec that will be played if both supplemental codec is su…

e7d22a6

…pported

parse multiple supplemental codec separated by white space

c5cb8c7

avoid unecessary call to iseCodecSupported if supplementalCodec is true

c57acb9

remove test file that shouldn't have been add

d14148b

myoann reviewed Nov 9, 2023

View reviewed changes

remplace == with isNullOrUndefined function

2d56d1a

peaBerberian reviewed Nov 9, 2023

View reviewed changes

src/parsers/manifest/dash/js-parser/node_parsers/utils.ts Outdated Show resolved Hide resolved

update regex to match more unexpected whitespace

3d7d1da

myoann reviewed Nov 9, 2023

View reviewed changes

src/parsers/manifest/dash/common/parse_representations.ts Outdated Show resolved Hide resolved

src/parsers/manifest/dash/js-parser/node_parsers/__tests__/utils.test.ts Outdated Show resolved Hide resolved

src/manifest/representation.ts Outdated Show resolved Hide resolved

Florent-Bouisset added 4 commits November 9, 2023 16:03

code review

49d1a7d

update rust WASM parser to parse supplementalCodecs

385b533

doc: update rfc number that obsoletes rfc4281

7bdef9d

move codec converting logic in "common" to convert from any parser

9010407

peaBerberian reviewed Nov 9, 2023

View reviewed changes

src/parsers/manifest/dash/js-parser/node_parsers/utils.ts Outdated Show resolved Hide resolved

code review

a02d3df

set the codec to supplementalCodec if supported and remove supplement…

0a694d7

…alCodec in the representation

handle case for subtitle

be795bc

peaBerberian reviewed Nov 10, 2023

View reviewed changes

Florent-Bouisset changed the title ~~[WIP][Proposal] scte214 - use supplemental codec instead of base codec if it's supported~~ [Proposal] scte214 - use supplemental codec instead of base codec if it's supported Nov 13, 2023

Florent-Bouisset added 2 commits November 13, 2023 10:13

move code to another folder

b16c914

reset unchanged file

435033d

remove staged file

99d3a3f

peaBerberian merged commit 491945b into master Nov 13, 2023
2 of 3 checks passed

peaBerberian deleted the feature-use-supplemental-codec-scte214 branch January 5, 2024 13:57

peaBerberian restored the feature-use-supplemental-codec-scte214 branch January 5, 2024 13:57

peaBerberian mentioned this pull request Jan 24, 2024

Release/v3.33.0 #1363

Merged

peaBerberian deleted the feature-use-supplemental-codec-scte214 branch February 7, 2024 17:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Proposal] scte214 - use supplemental codec instead of base codec if it's supported #1307

[Proposal] scte214 - use supplemental codec instead of base codec if it's supported #1307

Florent-Bouisset commented Oct 27, 2023

peaBerberian left a comment

Florent-Bouisset commented Nov 7, 2023

peaBerberian commented Nov 7, 2023 •

edited

Loading

myoann Nov 9, 2023 •

edited

Loading

Florent-Bouisset Nov 9, 2023

peaBerberian Nov 9, 2023 •

edited

Loading

Florent-Bouisset Nov 9, 2023 •

edited

Loading

peaBerberian Nov 9, 2023

peaBerberian Nov 9, 2023

Florent-Bouisset Nov 9, 2023

peaBerberian Nov 9, 2023

peaBerberian commented Nov 9, 2023

Florent-Bouisset commented Nov 10, 2023

peaBerberian commented Nov 10, 2023

peaBerberian Nov 10, 2023

peaBerberian commented Nov 10, 2023

peaBerberian commented Nov 13, 2023

[Proposal] scte214 - use supplemental codec instead of base codec if it's supported #1307

[Proposal] scte214 - use supplemental codec instead of base codec if it's supported #1307

Conversation

Florent-Bouisset commented Oct 27, 2023

Context

Implementation

Example

Work in progress

peaBerberian left a comment

Choose a reason for hiding this comment

Florent-Bouisset commented Nov 7, 2023

peaBerberian commented Nov 7, 2023 • edited Loading

myoann Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Florent-Bouisset Nov 9, 2023

Choose a reason for hiding this comment

peaBerberian Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Florent-Bouisset Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

peaBerberian Nov 9, 2023

Choose a reason for hiding this comment

peaBerberian Nov 9, 2023

Choose a reason for hiding this comment

Florent-Bouisset Nov 9, 2023

Choose a reason for hiding this comment

peaBerberian Nov 9, 2023

Choose a reason for hiding this comment

peaBerberian commented Nov 9, 2023

Florent-Bouisset commented Nov 10, 2023

peaBerberian commented Nov 10, 2023

peaBerberian Nov 10, 2023

Choose a reason for hiding this comment

peaBerberian commented Nov 10, 2023

peaBerberian commented Nov 13, 2023

peaBerberian commented Nov 7, 2023 •

edited

Loading

myoann Nov 9, 2023 •

edited

Loading

peaBerberian Nov 9, 2023 •

edited

Loading

Florent-Bouisset Nov 9, 2023 •

edited

Loading