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

Possibility of changing 'ceiling' to 'round' in EXT-X-TARGETDURATION algorithm #108

Open
sparrc opened this issue Jan 25, 2018 · 2 comments

Comments

@sparrc
Copy link

sparrc commented Jan 25, 2018

Hello,

I would like to propose changing the algorithm that determines the value of EXT-X-TARGETDURATION in HLS playlists from a ceiling round to a regular round, the relevant code is here:

m3u8/writer.go

Lines 307 to 309 in 6ab8f28

if p.TargetDuration < seg.Duration {
p.TargetDuration = math.Ceil(seg.Duration)
}

The reason is that Apple changed their recommendation for EXT-X-TARGETDURATION in revision 12 of the HLS spec to be greater than or equal EXTINF, when rounded to the nearest integer, see https://tools.ietf.org/html/draft-pantos-http-live-streaming-12#section-3.4.2

You can read the previous spec here: https://tools.ietf.org/html/draft-pantos-http-live-streaming-11#section-3.4.2

The current algorithm, which does a ceiling, does create playlists that conform to the spec.

BUT it can also arbitrarily create playlists with 11s target durations when the user requests 10s (when it would be perfectly valid to have a 10s target duration). Generating playlists that exactly match the spec recommendation is preferable for us, and can also help with scrubbing accuracy on a wider range of players.

I've opened this issue as more of a discussion because I understand that there might be some pushback given that the current behavior technically creates spec-compliant playlists. If there is any chance of this being activated as a feature flag this would also be acceptable for us.

I will submit a preliminary PR shortly that implements this.

@leikao
Copy link
Collaborator

leikao commented Jan 22, 2019

Hi @sparrc , sorry for late reply.

Apple changed their recommendation for EXT-X-TARGETDURATION in revision 12 of the HLS spec to be greater than or equal EXTINF, when rounded to the nearest integer.

Yes, the description of EXT-X-TARGETDURATION tag is changed in the latest HLS spec., so the following playlist (from Apple Developer) is correct now:

  #EXTM3U
  #EXT-X-TARGETDURATION:4
  #EXT-X-VERSION:6
  #EXT-X-PLAYLIST-TYPE:VOD
  #EXT-X-MAP:URI="VH"
  #EXTINF:4.004,
  V1andV2
  #EXTINF:4.004,
  V3andV4
  #EXTINF:2.002,
  V5
  #EXT-X-ENDLIST

Going back to the issue, I prefer to choose math.Ceil() instead of math.Round(), mainly in two reasons:

  1. the implementation is compliance with the spec.;
  2. the implementation is backward compatible.

In a word, current implement is more compatible with various HLS devices in the market.

@kapvode
Copy link

kapvode commented May 10, 2019

Would you consider fixing this at least for reading?

For example, the current implementation parses the above playlist as having a target duration of 5, which is different from the actual value in the playlist.

In my mind, if you think the playlist is incorrect, you could maybe return an error if the strict parsing option is turned on, but you should never modify the playlist you are reading.

When I was trying to find the code responsible for this, I was a bit surprised that it was in writer.go even though I was reading a playlist.

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 a pull request may close this issue.

3 participants