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

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

Merged
merged 20 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
7d1ecef
parse supplemental codec
Florent-Bouisset Oct 27, 2023
85b5941
add a property isSupplementalCoded supported and update getMimeType
Florent-Bouisset Oct 27, 2023
09e1d03
add documentation for isSupplementalCodecSupported property
Florent-Bouisset Nov 7, 2023
6c67e53
parse supplementalCodec in the adaptation set
Florent-Bouisset Nov 7, 2023
e7d22a6
return the codec that will be played if both supplemental codec is su…
Florent-Bouisset Nov 7, 2023
c5cb8c7
parse multiple supplemental codec separated by white space
Florent-Bouisset Nov 8, 2023
c57acb9
avoid unecessary call to iseCodecSupported if supplementalCodec is true
Florent-Bouisset Nov 8, 2023
d14148b
remove test file that shouldn't have been add
Florent-Bouisset Nov 8, 2023
2d56d1a
remplace == with isNullOrUndefined function
Florent-Bouisset Nov 9, 2023
3d7d1da
update regex to match more unexpected whitespace
Florent-Bouisset Nov 9, 2023
49d1a7d
code review
Florent-Bouisset Nov 9, 2023
385b533
update rust WASM parser to parse supplementalCodecs
Florent-Bouisset Nov 9, 2023
7bdef9d
doc: update rfc number that obsoletes rfc4281
Florent-Bouisset Nov 9, 2023
9010407
move codec converting logic in "common" to convert from any parser
Florent-Bouisset Nov 9, 2023
a02d3df
code review
Florent-Bouisset Nov 10, 2023
0a694d7
set the codec to supplementalCodec if supported and remove supplement…
Florent-Bouisset Nov 10, 2023
be795bc
handle case for subtitle
Florent-Bouisset Nov 10, 2023
b16c914
move code to another folder
Florent-Bouisset Nov 13, 2023
435033d
reset unchanged file
Florent-Bouisset Nov 13, 2023
99d3a3f
remove staged file
Florent-Bouisset Nov 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/manifest/representation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ class Representation {
*/
public codec : string | undefined;

/**
* Supplemental codecs are defined as backwards-compatible codecs enhancing
Florent-Bouisset marked this conversation as resolved.
Show resolved Hide resolved
* the experience of a base layer codec
*/
public supplementalCodec?: string | undefined;
peaBerberian marked this conversation as resolved.
Show resolved Hide resolved


public isSupplementalCodecSupported: boolean;
peaBerberian marked this conversation as resolved.
Show resolved Hide resolved
/**
* A string describing the mime-type for this Representation.
* Examples: audio/mp4, video/webm, application/mp4, text/plain
Expand Down Expand Up @@ -161,6 +169,10 @@ class Representation {
this.bitrate = args.bitrate;
this.codec = args.codecs;

if (args.supplementalCodecs !== undefined) {
this.supplementalCodec = args.supplementalCodecs;
}

if (args.isSpatialAudio !== undefined) {
this.isSpatialAudio = args.isSpatialAudio;
}
Expand Down Expand Up @@ -191,6 +203,15 @@ class Representation {

this.cdnMetadata = args.cdnMetadata;

if (this.supplementalCodec !== undefined) {
const supplementalCodecMimeTypeStr =
`${this.mimeType ?? ""};codecs="${this.supplementalCodec ?? ""}"`;
const isSupplementalCodecSupported = isCodecSupported(supplementalCodecMimeTypeStr);
peaBerberian marked this conversation as resolved.
Show resolved Hide resolved
this.isSupplementalCodecSupported = isSupplementalCodecSupported;
} else {
this.isSupplementalCodecSupported = false;
}

this.index = args.index;
if (opts.type === "audio" || opts.type === "video") {
const mimeTypeStr = this.getMimeTypeString();
Expand All @@ -210,7 +231,11 @@ class Representation {
* @returns {string}
*/
public getMimeTypeString() : string {
return `${this.mimeType ?? ""};codecs="${this.codec ?? ""}"`;
if (this.isSupplementalCodecSupported) {
return `${this.mimeType ?? ""};codecs="${this.supplementalCodec ?? ""}"`;
} else {
return `${this.mimeType ?? ""};codecs="${this.codec ?? ""}"`;
}
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/parsers/manifest/dash/common/parse_representations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link

@myoann myoann Nov 9, 2023

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

@peaBerberian peaBerberian Nov 9, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator Author

@Florent-Bouisset Florent-Bouisset Nov 9, 2023

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

As you want, both are good to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

supplementalCodecs = representation.attributes.supplementalCodecs;
} else if (adaptation.attributes.supplementalCodecs != null) {
supplementalCodecs = adaptation.attributes.supplementalCodecs;
}
if (supplementalCodecs != null) {
parsedRepresentation.supplementalCodecs = supplementalCodecs;
Florent-Bouisset marked this conversation as resolved.
Show resolved Hide resolved
}

if (representation.attributes.frameRate != null) {
parsedRepresentation.frameRate =
representation.attributes.frameRate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ function parseRepresentationAttributes(
dashName: "qualityRanking" });
break;

case "scte214:supplementalCodecs":
attributes.supplementalCodecs = attr.value;
break;

case "segmentProfiles":
attributes.segmentProfiles = attr.value;
break;
Expand Down
2 changes: 2 additions & 0 deletions src/parsers/manifest/dash/node_parser_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ export interface IAdaptationSetAttributes {
segmentAlignment? : number|boolean;
segmentProfiles? : string;
subsegmentAlignment? : number|boolean;
supplementalCodecs?: string;
width? : number;
availabilityTimeComplete?: boolean;
availabilityTimeOffset?: number;
Expand Down Expand Up @@ -271,6 +272,7 @@ export interface IRepresentationAttributes {
profiles? : string;
qualityRanking? : number;
segmentProfiles? : string;
supplementalCodecs?: string;
width? : number;
availabilityTimeComplete?: boolean;
availabilityTimeOffset?: number;
Expand Down
2 changes: 2 additions & 0 deletions src/parsers/manifest/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,8 @@ export interface IParsedRepresentation {
hdrInfo?: IHDRInformation | undefined;
/** `true` if audio has Dolby Atmos. */
isSpatialAudio?: boolean | undefined;

supplementalCodecs? : string | undefined;
}

/** Every possible types an Adaptation can have. */
Expand Down
35 changes: 35 additions & 0 deletions test/suppCodec.mpd
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?xml version="1.0"?>
Florent-Bouisset marked this conversation as resolved.
Show resolved Hide resolved
<MPD mediaPresentationDuration="PT12M14S" minBufferTime="PT3.00S"
profiles="urn:mpeg:dash:profile:isoff-live:2011" type="static"
xmlns="urn:mpeg:dash:schema:mpd:2011"
xmlns:scte214="urn:scte:dash:scte214-extensions">
<!-- Created with Bento4 mp4-dash.py, VERSION=1.6.0-588 -->
<Period>
<AdaptationSet lang="en" mimeType="audio/mp4" segmentAlignment="true" startWithSAP="1">
<SegmentTemplate duration="3000" initialization="https://www.bok.net/dash/tears_of_steel/cleartext/$RepresentationID$/init.mp4"
media="https://www.bok.net/dash/tears_of_steel/cleartext/$RepresentationID$/seg-$Number$.m4f" startNumber="1" timescale="1000" />
<Representation audioSamplingRate="44100" bandwidth="132348" codecs="mp4a.40.2" id="audio/en">
<AudioChannelConfiguration
schemeIdUri="urn:mpeg:dash:23003:3:audio_channel_configuration:2011" value="2" />
</Representation>
</AdaptationSet>
<AdaptationSet mimeType="video/mp4" segmentAlignment="true" startWithSAP="1">
<SegmentTemplate duration="3000" initialization="https://www.bok.net/dash/tears_of_steel/cleartext/$RepresentationID$/init.mp4"
media="https://www.bok.net/dash/tears_of_steel/cleartext/$RepresentationID$/seg-$Number$.m4f" startNumber="1" timescale="1000" />
<!-- <Representation bandwidth="686685" codecs="avc1.42C015" frameRate="24" height="214"
id="video/1" scanType="progressive" width="512" />
<Representation bandwidth="1116150" codecs="avc1.42C016" frameRate="24" height="294"
id="video/2" scanType="progressive" width="704" /> -->
<Representation bandwidth="686685" codecs="avc1.42C015"
scte214:supplementalCodecs="avc1.42C01F" scte214:supplementalProfiles="db1p" frameRate="24"
height="214"
id="video/3" scanType="progressive" width="512" />
<!-- <Representation bandwidth="1929169" codecs="avc1.42C01F" frameRate="24" height="428"
id="video/4" scanType="progressive" width="1024" />
<Representation bandwidth="2362822" codecs="avc1.42C01F" frameRate="24" height="482"
id="video/5" scanType="progressive" width="1152" />
<Representation bandwidth="2470094" codecs="avc1.4D401F" frameRate="24" height="534"
id="video/6" scanType="progressive" width="1280" /> -->
</AdaptationSet>
</Period>
</MPD>
Loading