-
Notifications
You must be signed in to change notification settings - Fork 0
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 CMAF ID3 metadata support #1
base: feature/cmaf-cc
Are you sure you want to change the base?
Conversation
Curious why you chose to make this ID3 specific and not just add support for generic EMSG v1 with any scheme ID URL? |
@johndeu I don't have a good reason other than the fact that this was what my company project needed and what I added to our own fork. How do I make it more generic? Is it just removing the scheme ID URL check or is there a more generic spec I should be looking at? I'm only aware of https://aomediacodec.github.io/id3-emsg/ and https://aomediacodec.github.io/id3-emsg/. |
@victor-at-work The AOM spec that we created with Apple really just uses the Common Media Application Framework file format (CMAF) with the Event Message Box 'emsg' v1. If you look at the implementation of the Event Message box in CMAF, it is meant to be very generic. It has a "scheme id URI", which is supposed to be a unique URI namespace which defines what is in the payload of the box. In addition it has the presentation time, duration, unique id, and a value property that can all be filled out as well. The goal is to be a generic box, that has a payload, that is defined by the scheme URI (consider it a namespace). For Apple, we wanted to define a specific URL that would be used to identify that the payload contained ID3v2 metadata. My comment is that we should not introduce code to HLS.js that precludes the ability to fire other custom defined events as well. |
@@ -205,7 +201,7 @@ export function parseVideoSegmentTextTrackSamples (data, videoTrackId) { | |||
}); | |||
} | |||
|
|||
export function parseVideoSegmentId3TrackSamples (data) { | |||
export function parseEmsg (data) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still no support for emsg v1 right? Can we someone add that to this PR?
https://aomediacodec.github.io/id3-emsg/
"One or more Event Message boxes (‘emsg’) [CMAF] can be included per segment. Version 1 of the Event Message box [DASH] must be used."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I understand what you're saying? This is v1 right?
@@ -8,6 +8,10 @@ import ID3 from '../demux/id3'; | |||
import { sendAddTrackEvent, clearCurrentCues, getClosestCue } from '../utils/texttrack-utils'; | |||
|
|||
const MIN_CUE_DURATION = 0.25; | |||
const VALID_ID3_SCHEME_ID_URIS = [ | |||
'https://aomedia.org/emsg/ID3', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why even include AOM ID3 scheme here if the v1 emsg box is not yet supported?
On reading the spec, the v0 emsg box uses relative timestamp (delta) v1 uses absolute timestamp 64-bit. Some sort of absolute to relative time mapping would be needed if the client receives the emsg data and map it back to the video time. |
This PR will...
Add ID3 metadata support for CMAF streams. This is dependent on video-dev#3016.
Why is this Pull Request needed?
ID3 metadata for CMAF is not currently supported.
Are there any points in the code the reviewer needs to double check?
Resolves issues:
video-dev#2360
Checklist