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

Add SUPPLEMENTAL-CODECS support in both DASH and HLS #1785

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ybai001
Copy link
Contributor

@ybai001 ybai001 commented Oct 8, 2024

Add SUPPLEMENTAL-CODECS support in both DASH and HLS.
-- HLS Specification: https://developer.apple.com/documentation/http-live-streaming/hls-authoring-specification-for-apple-devices-appendixes#The-SUPPLEMENTAL-CODECS-attribute
-- MPEG-DASH Specification: Let me know whether you need a copy of the specification. (See also Dash-Industry-Forum/dash.js#4585)

@tonihei
Copy link
Collaborator

tonihei commented Oct 11, 2024

Thanks for the contribution! This is definitely something we don't fully parse yet and were planning to add anyway.

I still need to look through some of the changes in more detail to provide comments, but one high-level one already: It would help to understand why the changes to Format are needed exactly. I was under the impression the codecs field directly and unambiguously maps to any fallback baseline codec that can be used instead, so specifying supplementalCodecs seems redundant? And the videoRange field (if needed) should probably be written into the existing Format.colorInfo.colorRange instead.

@ybai001
Copy link
Contributor Author

ybai001 commented Oct 14, 2024

Thanks for quick reply. I think you are right. I'll refine the code to remove the change of Format.java and update corresponding files.

According to Google's comments, removing the modification on Format.java since it is redundant.
Remove redundant blank line.
}
}
}
if (isDolbyVisionFormat(videoRange, codecs, supplementalCodecs, supplementalProfiles)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a problem here. Because I move this part of logic from HlsMediaPeriod.java to HlsPlaylistParser.java and the "codecs" is a string that includes both audio and video codec name, I can't update the codecs directly. I need to refine this part of code again.

If we handle the "codecs" in  HlsPlaylistParser.java rather than in  HlsMediaPeriod.java, since the "codecs" property includes both audio and video codec names here, we can't update it directly. Only video part should be updated by Dolby codec string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants