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 EXT-X-DATERANGE Metadata #4720

Merged
merged 1 commit into from Jul 11, 2022

Conversation

robwalch
Copy link
Collaborator

@robwalch robwalch commented Jun 2, 2022

This PR will...

Parse and validate Date Range tags in HLS playlists on playlist load. Date Range tags are converted to cues on the HTMLMediaElement's metadata text track on level update. In addition, the presence and duration of Date Range cues (as well as inband ID3 and emsg metadata cues already supported by HLS.js) are updated to reflect the Date Range content of the active playlist and inband metadata found in buffered audio/video tracks.

Why is this Pull Request needed?

These updates should more closely match the metadata text track cues produced by Safari when playing the same HLS stream natively in Safari.

Additional changes

  • Add hls.playingDate to API: gets program date time at media playhead (video.currentTime)
  • Fixed build regression by formatting code with prettier

Resolves issues:

Resolves #2218

Checklist

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • API or design changes are documented in API.md

@gkatsev
Copy link
Member

gkatsev commented Jun 3, 2022

Without testing and not high familiarity with hls.js's code, this seems reasonable to me. I think my main two points of thought are:

  • how strict does hls.js be with matching the spec? There's a lot of various MUSTs in the spec around when and how various tags are required, but I don't think there were any specific checks for some of these
  • Thought this was a good time to mention work in the W3C to bring back and standardize the DataCue: https://github.com/WICG/datacue

@robwalch
Copy link
Collaborator Author

robwalch commented Jun 3, 2022

Not a Contribution

how strict does hls.js be with matching the spec? There's a lot of various MUSTs in the spec around when and how various tags are required, but I don't think there were any specific checks for some of these

HLS.js is not a stream validation tool. We try to follow the spec, but generally favor performance and interoperability over strict compliance. It is the duty of the stream author to follow the spec. Not doing so may result in unexpected client behavior.

A few things we are not checking with this change request:

  • not validating the CUE attribute (not yet implemented)
  • not validating X-<client-attribute> names or value types
  • not validating SCTE35-<CMD|OUT|IN> (only parsing hex data to match Safari DataCue content)
  • not validating END-ON-NEXT overlap with DURATION or END-DATE:
 An EXT-X-DATERANGE tag with an END-ON-NEXT=YES attribute MUST NOT
   contain DURATION or END-DATE attributes.

That these rules are followed is expected not validated by HLS.js.

The unit tests should cover what we are validating, and the rule is to ignore invalid tags:

Clients SHOULD ignore EXT-X-DATERANGE tags with illegal syntax.

I think what has been added is pretty good in terms of preventing issues with bad updates related to duration and merging DATERANGES with the same ID. Let me know if there's anything not covered here that you think we should add.

@gkatsev
Copy link
Member

gkatsev commented Jun 3, 2022

Yeah, being looser in accepting things makes sense to me. Just wasn't sure how far hls.js takes it.

@itsjamie
Copy link
Collaborator

itsjamie commented Jun 3, 2022

Yeah, being looser in accepting things makes sense to me. Just wasn't sure how far hls.js takes it.

Recently (~2 years?), the main thing has been trying to maintain behaviour with Safari native <video> playback. Intention being (at least for me when reviewing) so it's simpler to keep the same video element API calls without having divergent behaviours.

@robwalch robwalch added this to the 1.2.0 milestone Jun 24, 2022
Resolves #2218
Other changes:
- Remove cues on buffer flush based on track type associated with metadata schema (video: emsg, audio: org.id3)
- Handle Delta Playlist Update Date Range updates
- Parse and validate DateRange tags
- Add hls.playingDate to API: gets program date time at media playhead (video.currentTime)
- Fix code formatting with prettier
@robwalch robwalch force-pushed the feature/date-range-metadata-texttrack-cues branch from 7a7acc5 to 370e00c Compare July 11, 2022 20:24
@robwalch robwalch merged commit 2be03e9 into master Jul 11, 2022
@robwalch robwalch deleted the feature/date-range-metadata-texttrack-cues branch July 11, 2022 21:11
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.

Support EXT-X-DATERANGE
3 participants