search

shaka-project / shaka-packager

A media packaging and development framework for VOD and Live DASH and HLS applications, supporting Common Encryption for Widevine and other DRM Systems.
https://shaka-project.github.io/shaka-packager/
Other
1.95k stars 503 forks source link

HLS target duration should be rounded to nearest integer #841

Open joeyparrish opened 3 years ago

joeyparrish commented 3 years ago

System info

Operating System: Ubuntu 20.04 LTS Shaka Packager Version: v2.4.1-c731217-release

Issue and steps to reproduce the problem

Packager Command: I used Shaka Streamer instead of using Packager directly.

Commands generated by Shaka Streamer:

What is the expected result? Segment durations are all roughly 4s (4.000s for video, 4.011 and 3.989 for audio). So I would expect the EXT-X-TARGET-DURATION to be 4, as well.

What happens instead? The target duration is 5, instead.

The HLS spec section 4.3.3.1 says:

The EXT-X-TARGETDURATION tag specifies the maximum Media Segment duration. The EXTINF duration of each Media Segment in the Playlist file, when rounded to the nearest integer, MUST be less than or equal to the target duration

kqyang commented 3 years ago

@joeyparrish Thanks for bringing it up! Agree that it is better to round to the nearest integer instead of rounding up to the next integer.

I'll consider it an "enhancement" as "rounding up" does not violate HLS specification.

JPeMu commented 3 years ago

@kqyang Any updates on this? Are you open to a pull request? Whilst it doesn't violate the HLS spec for sure, it's not expected behaviour for an HLS packager.

kqyang commented 3 years ago

@JPeMu Definitely. A PR is always welcomed and appreciated.

Canta commented 3 years ago

@joeyparrish I disagree with your interpretation.

By the standard, TARGETDURATION should be greater or equal. That discards rounding down, and in your examples "5" is actually the correct value.

SeawardT commented 3 years ago

@Canta is correct. Target Duration must be set to the largest value of any segment and then rounded up.

Canta commented 3 years ago

Let me add some context to my previous comment, as I feel it sounds kinda picky given that common sense dictates that 0.011 seconds shouldn't be an issue for any player, and thus rounding down is not a big deal.

I stumbled into this issue while looking for open issues regarding TARGETDURATION tag. That's because I'm having trouble with it. Take a look at this:

[root@server ~]# grep TARGETDURATION /path/to/file.m3u8
#EXT-X-TARGETDURATION:11
[root@server ~]# grep EXTINF /path/to/file.m3u8 | sort -u
#EXTINF:3.200,

I expect TARGETDURATION to be 4 there. Yet, it's 11, because apparently at some time there was some abnormally large chunk in the playlist, and TARGETDURATION doesn't seem to refresh from the historical maximum detected. Thing is, I've discovered it because some players in some apps in some devices where crashing because of this.

My problem is different. But my point is: please just do as the spec says. Players can be and actually are a massive PITA for us working on the server side, and strictly respecting the standard is usually our only line of defense when dealing with other parties involved in multimedia operations. If tomorrow Shaka Packager deviates from the standard just for that annoying 0.011 seconds, then I'll most likely have to edit packager's code just for convincing some other party that their player is having an issue. So, please, if possible, just don't deviate from the standard by default, no matter how nonsensical it could look.

Maybe allowing this by command line flag would be a good alternative.

bgeiser commented 3 years ago

@Canta is correct. Target Duration must be set to the largest value of any segment and then rounded up.

Hmm... the above quoted section (4.3.3.1) in the spec says when rounded to the nearest integer and not when rounded up to the next integer. So, IMHO, 4 should be correct in the above example. Of course, 5 is not wrong either and maybe a safer choice, right?

Canta commented 3 years ago

Well... now I'm confused. :P

But there's two values involved here:

  1. EXT-X-TARGETDURATION
  2. EXTINF

EXTINF is the one with the MUST restriction. And that MUST says "less than or equal". So, if we round down EXT-X-TARGETDURATION for whatever reason, and we decide to also round EXTINF, that second rounding should be less or equal than the first one.

In my previous comments I wasn't considering rounding EXTINF. My argument was "targetduration should not be 4 for chunk duration 4.011". But if EXTINF is also rounded to 4 for that chunk, I guess it's OK to also round targetduration. I believe it's still safer to just round up targetduration, and that's it. But it's a valid point.

JPeMu commented 3 years ago

For a chunk duration of 4.011 its perfectly acceptable (and even desirable) in my experience to use a target duration of 4.0 and remain spec compliant. Apple's HLS validator tool accepts this too.

The spec states rounded to the nearest integer, not rounded up.

Just my 2cents.

On Thu, 10 Jun 2021, 22:41 Daniel Cantarín, @.***> wrote:

Well... now I'm confused. :P

But there's two values involved here:

  1. EXT-X-TARGETDURATION
  2. EXTINF

EXTINF is the one with the MUST restriction. And that MUST says "less than or equal". So, if we round down EXT-X-TARGETDURATION for whatever reason, and we decide to also round EXTINF, that second rounding should be less or equal than the first one.

In my previous comments I wasn't considering rounding EXTINF. My argument was "targetduration should not be 4 for chunk duration 4.011". But if EXTINF is also rounded to 4 for that chunk, I guess it's OK to also round targetduration. I believe it's still safer to just round up targetduration, and that's it. But it's a valid point.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/google/shaka-packager/issues/841#issuecomment-859095121, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB34C7FUVDHHTDZ6R5HA65DTSEWP5ANCNFSM4RMEZYPA .

Canta commented 3 years ago

For a chunk duration of 4.011 its perfectly acceptable (and even desirable) in my experience to use a target duration of 4.0 and remain spec compliant. Apple's HLS validator tool accepts this too.

I believe you. And I also share the belief that it's desirable (if I don't get annoying with the spec).

The spec states rounded to the nearest integer, not rounded up.

That goes for EXTINF, not for EXT-X-TARGETDURATION. And, actually, it's not even for EXTINF, at least not in the quote.

The "MUST" clause involves a relationship between the two (different) values. The spec quote mentions the hypothetical case when you round to the nearest integer. The restriction goes like this: "whatever your rounding, EXTINF should never be bigger than EXT-X-TARGETDURATION".

So... what you say should only be ok if you ALSO round EXT-X-TARGETDURATION, and not JUST EXT-X-TARGETDURATION.

dev4justice commented 2 weeks ago

I've been experiencing this issue as well. According to the spec, there’s a 0.5 second "leeway":

7.7. Media Segments MUST NOT exceed the target duration by more than 0.5 seconds.

Therefore, we should only round up #EXT-X-TARGETDURATION when #EXTINF > (#EXT-X-TARGETDURATION + 0.5). I'm not entirely sure if this should be >= or just >, but currently, we're rounding up even for a fraction of it.

The validator accepts this change if I manually round down the TARGETDURATION.

Additionally, there's another issue related to TARGETDURATION, this time with the iframe playlists. The options --segment_duration=6 --fragment_duration=2 should result in the video playlists' target duration being 6 and the iframe's target duration being 2.

However, since my audio-only playlists never round to an integer (I would appreciate some help with this), the packager sets the target duration of all playlists (audio, video, iframe) to 7. This causes the validation report to show a -50% diff errors for iframe variants. Manually updating the audio and video target durations to 6 and the iframe's target duration to 2 results in ~0% differences.