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 support for emsg ID3 metadata in fmp4 segments #4458

Merged
merged 1 commit into from
Jan 18, 2022

Conversation

robwalch
Copy link
Collaborator

@robwalch robwalch commented Dec 14, 2021

This PR will...

Add support for emsg timed metadata within fragmented MP4.

These changes allow for timed metadata delivery (ID3 via emsg) for use cases such as content labeling and advertising in CMAF streams in the same way HLS.js currently supports ID3 in other container formats.

HTMLMediaElement text track and cues generated by HLS.js are similar in timing and content to those added with native HLS playback in Safari.

Supported schemas include any url containing "/emsg/ID3" or "/emsg-id3" (case insensitive).

Sample test streams from #2360:

Resolves issues:

Closes #2360

Checklist

  • changes have been done against master branch, and PR does not conflict

@robwalch robwalch added this to the 1.2.0 milestone Dec 14, 2021
@robwalch robwalch mentioned this pull request Dec 15, 2021
1 task
@robwalch robwalch force-pushed the feature/emsg-id3-support branch 2 times, most recently from 3e59a8a to 7764d9a Compare December 16, 2021 20:27
@robwalch robwalch added this to Done in Metadata Dec 17, 2021
@robwalch robwalch moved this from Done to In progress in Metadata Dec 17, 2021
@cjpillsbury
Copy link
Collaborator

Adding reference link for data structure for reference/posterity (link in the original issue is incorrect or out of date):
https://aomediacodec.github.io/id3-emsg/#id3-metadata-in-an-event-message-box

Copy link
Collaborator

@cjpillsbury cjpillsbury left a comment

Choose a reason for hiding this comment

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

Still going to do some testing, but on a code only review looks 👌 so far other than a few minor asks (defer to you on whether you think the changes are necessary).

} from '../utils/mp4-tools';
import { dummyTrack } from './dummy-demuxed-track';
import type { HlsEventEmitter } from '../events';
import type { HlsConfig } from '../config';

const emsgSchemePattern = /\/emsg[-/]ID3/i;
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure about best practices here, but are we using this looser match to handle both "https://aomedia.org/emsg/ID3" (defined in: https://aomediacodec.github.io/av1-id3/) and the outdated "https://developer.apple.com/streaming/emsg-id3"? If so, should we be more explicit with those two matches for potential future-proofing concerns?

Copy link
Collaborator Author

@robwalch robwalch Jan 11, 2022

Choose a reason for hiding this comment

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

Not a Contribution

For the purposes of this contribution, I wanted the broadest support for available test content, but could not explicitly add support for undocumented schemes. Perhaps other players are even less strict that this?

src/utils/mp4-tools.ts Outdated Show resolved Hide resolved

timeScale = readUint32(data, 12);
presentationTimeDelta = readUint32(data, 16);
eventDuration = readUint32(data, 20);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this is unused in our codebase, but should we make eventDuration = undefined or NaN or some other special value in cases where its value is 0xFFFFFFFF for clarity? From the spec:

In general, ID3 don’t carry a duration and in those cases the event_duration field should be set to 0xFFFFFFFF. If in a particular case, the ID3 message carries a duration, it should be reflected in the event_duration field.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a Contribution

Since it is part of the spec, it is included for future use. It would help to have sample content with event durations defined. For everything else we have an open issue with some discussion on the subject #3879 (duration=Infinity until next cue).

id3Track,
timeOffset,
id3InitPts,
id3InitPts
Copy link
Collaborator

Choose a reason for hiding this comment

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

not having enough context here, wondering why we're using pts as dts here? Worried we might get some cross-browser inconsistencies with timings if we're not careful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a Contribution

The use of both PTS and DTS here is legacy code for dealing with MPEG-TS 33-bit timestamp rollover (this is not new code, just code I moved into flushTextTrackMetadataCueSamples). Since we can expect all emsg samples to have identical pts and dts, we can use the same value for both inputs.

cjpillsbury
cjpillsbury previously approved these changes Jan 11, 2022
Copy link
Collaborator

@cjpillsbury cjpillsbury left a comment

Choose a reason for hiding this comment

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

Basic smoke testing using the test playlists looks good. Consider this pre-approved, with your discretion on resolving any of the outstanding questions/asks.

@robwalch robwalch merged commit 38b5c1d into master Jan 18, 2022
Metadata automation moved this from In progress to Done Jan 18, 2022
@robwalch robwalch deleted the feature/emsg-id3-support branch January 18, 2022 22:14
@erankor
Copy link
Collaborator

erankor commented Jun 24, 2022

Hi,

We really want to use this feature, but from what I see it's not on the latest tag (v1.1.5), is there any ETA for the next release?

Thanks!

Eran

@petronioamaral
Copy link

Hello there,

great feature, is there any ETA for the next release?

Thanks
Petronio

@robwalch
Copy link
Collaborator Author

Hi @erankor and @petronioamaral,

This feature is in the latest beta release: v1.2.0-beta.1. If you have time next week, please try it out next week and provide feedback. v1.2.0 will be released soon.

@petronioamaral
Copy link

Hi @erankor and @petronioamaral,

This feature is in the latest beta release: v1.2.0-beta.1. If you have time next week, please try it out next week and provide feedback. v1.2.0 will be released soon.

Hi @robwalch I tested here and seems work very well, let us know when will be merge with the v1.2.0.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Inband Timed Metadata for FMP4 (ID3 within Emsgv1)
4 participants