Add support for scte214:supplementalCodecs#4585
Merged
dsilhavy merged 6 commits intoDash-Industry-Forum:developmentfrom Oct 31, 2024
Merged
Add support for scte214:supplementalCodecs#4585dsilhavy merged 6 commits intoDash-Industry-Forum:developmentfrom
dsilhavy merged 6 commits intoDash-Industry-Forum:developmentfrom
Conversation
This was referenced Oct 8, 2024
dsilhavy
requested changes
Oct 8, 2024
Collaborator
|
Another comment: |
Contributor
Author
|
Yes, these are good points that I brought up with the author of the scte text.
It’s not clear if multiple listed codecs are alternatives, or supposed to be paired with multiple codecs listed in the @codecs attribute.
I was told that for profiles that do not permit multiple codecs in the @codecs attribute, there will not be multiple codecs in the @scte214:supplementalCodecs attribute. After some investigation, dash-if iop is the only place where this is true. However, it’s generally the preferred behaviour.
At this stage, it might be easier to throw an error (or ignore @scte214:supplementalCodecs) if multiple codecs are listed.
… On 8 Oct 2024, at 22:01, Daniel Silhavy ***@***.***> wrote:
Another comment: @scte214:supplementalCodecs is a space separated list of codec strings. I assume we need to do an individual check for each of these strings to find out it a supplemental codec is supported. But then the question remains with which codec string we initialize the MSE Sourcebuffers
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Contributor
Author
|
Ah, yes. That part of the patch is missing.
… On 8 Oct 2024, at 21:56, Daniel Silhavy ***@***.***> wrote:
@dsilhavy requested changes on this pull request.
In src/streaming/utils/CapabilitiesFilter.js:
> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
}
return supported
});
+
+ // handle scte214:supplementalCodecs
+ as.Representation = as.Representation.map((rep) => {
+ const supplementalCodecs = rep['scte214:supplementalCodecs']
Please put the string scte214:supplementalCodecs into DashConstants
In src/streaming/utils/CapabilitiesFilter.js:
> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
}
return supported
});
+
+ // handle scte214:supplementalCodecs
+ as.Representation = as.Representation.map((rep) => {
+ const supplementalCodecs = rep['scte214:supplementalCodecs']
+ if (!supplementalCodecs) {
+ return rep
+ }
+ const codec = rep.mimeType + ';codecs="' + supplementalCodecs + '"';
+ const config = _createConfiguration(type, rep, codec);
+ const supported = capabilities.isCodecSupportedBasedOnTestedConfigurations(config, type);
For this new configuration, capabilities.runCodecSupportCheck is never executed before. As a consequence testedCodecConfigurations in Capabilities should not contain this new configuration and the resulting variable supported should never be true. See also the description of the isCodecSupportedBasedOnTestedConfigurations function:
Checks whether a codec is supported according to the previously tested configurations.
Note that you need to call runCodecSupportCheck() first to populate the testedCodecConfigurations array.
This function only validates codec support based on previously tested configurations.
In src/streaming/utils/CapabilitiesFilter.js:
> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
}
return supported
});
+
+ // handle scte214:supplementalCodecs
I would prefer to not have a second iteration over the Representations. I think what we should do:
• In filterUnsupportedFeatures execute runCodecSupportCheck with the "default config" but also the config for the scte214:supplementalCodecs string. After that, Capabilities.testedCodecConfigurations should contain the result for both configurations
• In CapabilitiesFilter._filterUnsupportedRepresentationsOfAdaptation check if at least one of the two valid codec configurations is supported. I would also prefer to not overwrite the codecs attribute of rep.
In src/streaming/utils/CapabilitiesFilter.js:
> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
}
return supported
});
+
+ // handle scte214:supplementalCodecs
+ as.Representation = as.Representation.map((rep) => {
+ const supplementalCodecs = rep['scte214:supplementalCodecs']
+ if (!supplementalCodecs) {
+ return rep
+ }
+ const codec = rep.mimeType + ';codecs="' + supplementalCodecs + '"';
Put this into a function in dashAdapter: getSupplementalCodec.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Contributor
Author
|
Actually, just missing from the MR.
… On 15 Oct 2024, at 15:35, Gregory McGarry ***@***.***> wrote:
Ah, yes. That part of the patch is missing.
> On 8 Oct 2024, at 21:56, Daniel Silhavy ***@***.***> wrote:
>
>
> @dsilhavy requested changes on this pull request.
> In src/streaming/utils/CapabilitiesFilter.js:
>> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
> }
> return supported
> });
> +
> + // handle scte214:supplementalCodecs
> + as.Representation = as.Representation.map((rep) => {
> + const supplementalCodecs = rep['scte214:supplementalCodecs']
>
> Please put the string scte214:supplementalCodecs into DashConstants
> In src/streaming/utils/CapabilitiesFilter.js:
>> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
> }
> return supported
> });
> +
> + // handle scte214:supplementalCodecs
> + as.Representation = as.Representation.map((rep) => {
> + const supplementalCodecs = rep['scte214:supplementalCodecs']
> + if (!supplementalCodecs) {
> + return rep
> + }
> + const codec = rep.mimeType + ';codecs="' + supplementalCodecs + '"';
> + const config = _createConfiguration(type, rep, codec);
> + const supported = capabilities.isCodecSupportedBasedOnTestedConfigurations(config, type);
>
> For this new configuration, capabilities.runCodecSupportCheck is never executed before. As a consequence testedCodecConfigurations in Capabilities should not contain this new configuration and the resulting variable supported should never be true. See also the description of the isCodecSupportedBasedOnTestedConfigurations function:
> Checks whether a codec is supported according to the previously tested configurations.
> Note that you need to call runCodecSupportCheck() first to populate the testedCodecConfigurations array.
> This function only validates codec support based on previously tested configurations.
>
> In src/streaming/utils/CapabilitiesFilter.js:
>> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
> }
> return supported
> });
> +
> + // handle scte214:supplementalCodecs
>
> I would prefer to not have a second iteration over the Representations. I think what we should do:
> • In filterUnsupportedFeatures execute runCodecSupportCheck with the "default config" but also the config for the scte214:supplementalCodecs string. After that, Capabilities.testedCodecConfigurations should contain the result for both configurations
> • In CapabilitiesFilter._filterUnsupportedRepresentationsOfAdaptation check if at least one of the two valid codec configurations is supported. I would also prefer to not overwrite the codecs attribute of rep.
> In src/streaming/utils/CapabilitiesFilter.js:
>> @@ -136,6 +136,22 @@ function CapabilitiesFilter() {
> }
> return supported
> });
> +
> + // handle scte214:supplementalCodecs
> + as.Representation = as.Representation.map((rep) => {
> + const supplementalCodecs = rep['scte214:supplementalCodecs']
> + if (!supplementalCodecs) {
> + return rep
> + }
> + const codec = rep.mimeType + ';codecs="' + supplementalCodecs + '"';
>
> Put this into a function in dashAdapter: getSupplementalCodec.
> —
> Reply to this email directly, view it on GitHub, or unsubscribe.
> You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Collaborator
Thanks for checking. Can you please add a check for multiple codecs then. If multiple codecs are present we should ignore the attribute but log a warning. |
…ith ensure the representation is accepted.
…nly the first is selected.
dsilhavy
approved these changes
Oct 31, 2024
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Let me know if you need a copy of the spec. With this change, DolbyVision Profile 8 now works.
For example, on Windows11 with DolbyVision: