Invalid EXT-X-TARGETDURATION in HLS

[Artem Skoretskiy]

Summary of the bug:

When I convert MP4 file to HLS (.m3u8 + .ts files), in some cases I get invalid #EXT-X-TARGETDURATION value.

In my case, I provide duration = 4, but have 5 in the file.

How to reproduce:

1. Get source file

ffmpeg -hide_banner -y -ss 00:01:00 -i http://ftp.nluug.nl/pub/graphics/blender/demo/movies/ToS/tearsofsteel_4k.mov -to 00:01:00 -vn -b:a 80k -c:a libfdk_aac file.mp4

Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 'http://ftp.nluug.nl/pub/graphics/blender/demo/movies/ToS/tearsofsteel_4k.mov':
  Metadata:
    major_brand     : qt  
    minor_version   : 512
    compatible_brands: qt  
    encoder         : Lavf54.29.104
  Duration: 00:12:14.00, start: 0.000000, bitrate: 73434 kb/s
    Stream #0:0(eng): Video: h264 (High) (avc1 / 0x31637661), yuv420p, 3840x1714 [SAR 1:1 DAR 1920:857], 73244 kb/s, 24 fps, 24 tbr, 24 tbn, 48 tbc (default)
    Metadata:
      handler_name    : DataHandler
      encoder         : libx264
    Stream #0:1(eng): Audio: aac (LC) (mp4a / 0x6134706D), 44100 Hz, stereo, fltp, 182 kb/s (default)
    Metadata:
      handler_name    : DataHandler
Stream mapping:
  Stream #0:1 -> #0:0 (aac (native) -> aac (libfdk_aac))
Press [q] to stop, [?] for help
Output #0, mp4, to 'file.mp4':
  Metadata:
    major_brand     : qt  
    minor_version   : 512
    compatible_brands: qt  
    encoder         : Lavf57.75.100
    Stream #0:0(eng): Audio: aac (libfdk_aac) (mp4a / 0x6134706D), 44100 Hz, stereo, s16, 80 kb/s (default)
    Metadata:
      handler_name    : DataHandler
      encoder         : Lavc57.100.104 libfdk_aac
size=     598kB time=00:01:00.00 bitrate=  81.6kbits/s speed=0.917x     
video:0kB audio:587kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 1.845107%

2. Generate HLS out of it

ffmpeg -loglevel info -i file.mp4 -c:a copy -hls_time 4 -hls_flags single_file -hls_list_size 0 file.m3u8

ffmpeg version N-86781-gd8f1982 Copyright (c) 2000-2017 the FFmpeg developers
  built with gcc 4.9.2 (Debian 4.9.2-10)
  configuration: --disable-shared --enable-static --enable-network --disable-ffplay --disable-ffserver --disable-debug --enable-logging --enable-avresample --enable-chromaprint --enable-bzlib --enable-frei0r --enable-gcrypt --enable-gmp --enable-gnutls --enable-gpl --enable-gray --enable-hardcoded-tables --enable-iconv --enable-ladspa --enable-libass --enable-libbluray --enable-libbs2b --enable-libcaca --enable-libcdio --enable-libcelt --enable-libdc1394 --enable-libfdk-aac --enable-libflite --enable-libfontconfig --enable-libfreetype --enable-libfribidi --enable-libgme --enable-libgsm --enable-libiec61883 --enable-libilbc --enable-libkvazaar --enable-libmodplug --enable-libmp3lame --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopencv --enable-libopenh264 --enable-libopenjpeg --enable-libopus --enable-libpulse --enable-librtmp --enable-librubberband --enable-libshine --enable-libsmbclient --enable-libsnappy --enable-libsoxr --enable-libspeex --enable-libssh --enable-libtesseract --enable-libtheora --enable-libtwolame --enable-libv4l2 --enable-libvidstab --enable-libvo-amrwbenc --enable-libvorbis --enable-libvpx --enable-libwavpack --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxavs --enable-libxcb --enable-libxcb-shape --enable-libxcb-shm --enable-libxcb-xfixes --enable-libxvid --enable-libzimg --enable-libzmq --enable-libzvbi --enable-lzma --enable-nonfree --enable-openal --enable-opengl --enable-openssl --enable-pthreads --enable-sdl2 --enable-vaapi --enable-vdpau --enable-version3 --enable-xlib --enable-zlib
  libavutil      55. 67.100 / 55. 67.100
  libavcodec     57.100.104 / 57.100.104
  libavformat    57. 75.100 / 57. 75.100
  libavdevice    57.  7.100 / 57.  7.100
  libavfilter     6. 95.100 /  6. 95.100
  libavresample   3.  6.  0 /  3.  6.  0
  libswscale      4.  7.101 /  4.  7.101
  libswresample   2.  8.100 /  2.  8.100
  libpostproc    54.  6.100 / 54.  6.100
Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 'file.mp4':
  Metadata:
    major_brand     : isom
    minor_version   : 512
    compatible_brands: isomiso2mp41
    encoder         : Lavf57.75.100
  Duration: 00:01:00.05, start: 0.000000, bitrate: 81 kb/s
    Stream #0:0(eng): Audio: aac (LC) (mp4a / 0x6134706D), 44100 Hz, stereo, fltp, 80 kb/s (default)
    Metadata:
      handler_name    : SoundHandler
[hls @ 0x2da3e00] Opening 'file.ts' for writing
[mpegts @ 0x2db8060] frame size not set
Output #0, hls, to 'file.m3u8':
  Metadata:
    major_brand     : isom
    minor_version   : 512
    compatible_brands: isomiso2mp41
    encoder         : Lavf57.75.100
    Stream #0:0(eng): Audio: aac (LC) (mp4a / 0x6134706D), 44100 Hz, stereo, fltp, 80 kb/s (default)
    Metadata:
      handler_name    : SoundHandler
Stream mapping:
  Stream #0:0 -> #0:0 (copy)
Press [q] to stop, [?] for help
[hls @ 0x2da3e00] Opening 'file.m3u8.tmp' for writing
    Last message repeated 14 times
[hls @ 0x2da3e00] Opening 'file.m3u8.tmp' for writing
size=N/A time=00:00:59.97 bitrate=N/A speed=1.15e+03x    
video:0kB audio:587kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown

3. Check results:

grep EXT-X-TARGETDURATION file.m3u8

#EXT-X-TARGETDURATION:5

Expected result:

grep EXT-X-TARGETDURATION file.m3u8

#EXT-X-TARGETDURATION:5

[Artem Skoretskiy]

I gave it another try with separate chunks:

ffmpeg -loglevel info -i file.mp4 -c:a copy -hls_time 4 -hls_list_size 0 file.m3u8

Then I've checked the duration of every chunk with ffprobe:

  Duration: 00:00:04.02, start: 1.400000, bitrate: 89 kb/s
  Duration: 00:00:03.88, start: 5.417056, bitrate: 91 kb/s
  Duration: 00:00:03.90, start: 9.410889, bitrate: 89 kb/s
  Duration: 00:00:03.88, start: 13.404722, bitrate: 90 kb/s
  Duration: 00:00:03.92, start: 17.421778, bitrate: 89 kb/s
  Duration: 00:00:03.92, start: 21.415600, bitrate: 89 kb/s
  Duration: 00:00:03.88, start: 25.409433, bitrate: 91 kb/s
  Duration: 00:00:03.88, start: 29.403267, bitrate: 91 kb/s
  Duration: 00:00:03.88, start: 33.420322, bitrate: 91 kb/s
  Duration: 00:00:03.85, start: 37.414156, bitrate: 92 kb/s
  Duration: 00:00:03.85, start: 41.407989, bitrate: 91 kb/s
  Duration: 00:00:03.90, start: 45.401822, bitrate: 90 kb/s
  Duration: 00:00:03.85, start: 49.418867, bitrate: 92 kb/s
  Duration: 00:00:03.92, start: 53.412700, bitrate: 89 kb/s
  Duration: 00:00:03.90, start: 57.406533, bitrate: 89 kb/s
  Duration: 00:00:00.02, start: 61.400367, bitrate: 388 kb/s

Maybe the reason is that first chunk is > 4 seconds and got rounded to 5.

Regarding the code:

Function that generates the text:

static void write_m3u8_head_block(HLSContext *hls, AVIOContext *out, int version,
                                  int target_duration, int64_t sequence)
{
    avio_printf(out, "#EXTM3U\n");
    avio_printf(out, "#EXT-X-VERSION:%d\n", version);
    if (hls->allowcache == 0 || hls->allowcache == 1) {
        avio_printf(out, "#EXT-X-ALLOW-CACHE:%s\n", hls->allowcache == 0 ? "NO" : "YES");
    }
    avio_printf(out, "#EXT-X-TARGETDURATION:%d\n", target_duration);
    avio_printf(out, "#EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", sequence);
    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
        avio_printf(out, "#EXT-X-MAP:URI=\"%s\"\n", hls->fmp4_init_filename);
    }
    av_log(hls, AV_LOG_VERBOSE, "EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", sequence);
}

Code that change the duration:

    for (en = hls->segments; en; en = en->next) {
        if (target_duration <= en->duration)
            target_duration = get_int_from_double(en->duration);
    }

[Steven Liu]

Replying to tonn81:

I gave it another try with separate chunks:

ffmpeg -loglevel info -i file.mp4 -c:a copy -hls_time 4 -hls_list_size 0 file.m3u8

Then I've checked the duration of every chunk with ffprobe:

  Duration: 00:00:04.02, start: 1.400000, bitrate: 89 kb/s

the duration is 5 when the *segment duration* big than the 4, eg: 4.02 is biger than 4.000000;
if the *segment duration* == 4, then the EXT-X-TARGETDURATION will 4.

Duration: 00:00:03.88, start: 5.417056, bitrate: 91 kb/s
Duration: 00:00:03.90, start: 9.410889, bitrate: 89 kb/s
Duration: 00:00:03.88, start: 13.404722, bitrate: 90 kb/s
Duration: 00:00:03.92, start: 17.421778, bitrate: 89 kb/s
Duration: 00:00:03.92, start: 21.415600, bitrate: 89 kb/s
Duration: 00:00:03.88, start: 25.409433, bitrate: 91 kb/s
Duration: 00:00:03.88, start: 29.403267, bitrate: 91 kb/s
Duration: 00:00:03.88, start: 33.420322, bitrate: 91 kb/s
Duration: 00:00:03.85, start: 37.414156, bitrate: 92 kb/s
Duration: 00:00:03.85, start: 41.407989, bitrate: 91 kb/s
Duration: 00:00:03.90, start: 45.401822, bitrate: 90 kb/s
Duration: 00:00:03.85, start: 49.418867, bitrate: 92 kb/s
Duration: 00:00:03.92, start: 53.412700, bitrate: 89 kb/s
Duration: 00:00:03.90, start: 57.406533, bitrate: 89 kb/s
Duration: 00:00:00.02, start: 61.400367, bitrate: 388 kb/s

}}}

Maybe the reason is that first chunk is > 4 seconds and got rounded to 5.

Regarding the code:

Function that generates the text:

static void write_m3u8_head_block(HLSContext *hls, AVIOContext *out, int version,
                                  int target_duration, int64_t sequence)
{
    avio_printf(out, "#EXTM3U\n");
    avio_printf(out, "#EXT-X-VERSION:%d\n", version);
    if (hls->allowcache == 0 || hls->allowcache == 1) {
        avio_printf(out, "#EXT-X-ALLOW-CACHE:%s\n", hls->allowcache == 0 ? "NO" : "YES");
    }
    avio_printf(out, "#EXT-X-TARGETDURATION:%d\n", target_duration);
    avio_printf(out, "#EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", sequence);
    if (hls->segment_type == SEGMENT_TYPE_FMP4) {
        avio_printf(out, "#EXT-X-MAP:URI=\"%s\"\n", hls->fmp4_init_filename);
    }
    av_log(hls, AV_LOG_VERBOSE, "EXT-X-MEDIA-SEQUENCE:%"PRId64"\n", sequence);
}

Code that change the duration:

    for (en = hls->segments; en; en = en->next) {
        if (target_duration <= en->duration)
            target_duration = get_int_from_double(en->duration);
    }

[Steven Liu]

The duration is correct

[Artem Skoretskiy]

I would disagree with @stevenliu -- result generated by ffmpeg is not what is expected -- TARGETDURATION does not match the value given as --hls_time.

Actually, there are two bugs in ffmpeg. Together they give this false result.

Here are the bugs:

1. ffmpeg generate HLS chunks with duration that does not match given --hls_time

ffmpeg should generate chunks not longer than given with --hls_time. For --hls_time 4 it generated 4.02 chunk instead of 4.0 or less.

2. ffmpeg converts chunk duration into EXT-X-TARGETDURATION incorrectly

According to official HLS validator (mediastreamvalidator), there could be 25% difference between actual chunk size and value. That means we should not ceil the value but rather round to nearest integer (so that delta between maximum chunk duration and TARGETDURATION is minimal). Then 4.02 would become 4, not 5.

I could file another bug report for HLS chunks generation if needed.

[Steven Liu]

Replying to tonn81:

I would disagree with @stevenliu -- result generated by ffmpeg is not what is expected -- TARGETDURATION does not match the value given as --hls_time.

Actually, there are two bugs in ffmpeg. Together they give this false result.

Here are the bugs:

1. ffmpeg generate HLS chunks with duration that does not match given --hls_time

ffmpeg should generate chunks not longer than given with --hls_time. For --hls_time 4 it generated 4.02 chunk instead of 4.0 or less.

Write a full frame is better than half frame. for example, the frame duration is 40, the frame sequence is: IPPIPPIPP, if the hls_time is set to 100ms. Do you mean it should split IP and half of next P and leave the other half to the next segment? Or leave the full P frame to the next segment start frame to make sure the hls_time 0.100?

2. ffmpeg converts chunk duration into EXT-X-TARGETDURATION incorrectly

According to official HLS validator (mediastreamvalidator), there could be 25% difference between actual chunk size and value. That means we should not ceil the value but rather round to nearest integer (so that delta between maximum chunk duration and TARGETDURATION is minimal). Then 4.02 would become 4, not 5.

Reference Spec:

​https://tools.ietf.org/html/draft-pantos-http-live-streaming-22#page-49

4.3.3.1. EXT-X-TARGETDURATION

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; longer segments can trigger playback stalls
or other errors. It applies to the entire Playlist file. Its format
is:

#EXT-X-TARGETDURATION:<s>

where s is a decimal-integer indicating the target duration in
seconds. The EXT-X-TARGETDURATION tag is REQUIRED.

I could file another bug report for HLS chunks generation if needed.

[Artem Skoretskiy]

Replying to stevenliu:

ffmpeg should generate chunks not longer than given with --hls_time. For --hls_time 4 it generated 4.02 chunk instead of 4.0 or less.

Write a full frame is better than half frame. for example, the frame duration is 40, the frame sequence is: IPPIPPIPP, if the hls_time is set to 100ms. Do you mean it should split IP and half of next P and leave the other half to the next segment? Or leave the full P frame to the next segment start frame to make sure the hls_time 0.100?

I cannot tell here. I just see that chunk duration is longer than requested.

2. ffmpeg converts chunk duration into EXT-X-TARGETDURATION incorrectly

According to official HLS validator (mediastreamvalidator), there could be 25% difference between actual chunk size and value. That means we should not ceil the value but rather round to nearest integer (so that delta between maximum chunk duration and TARGETDURATION is minimal). Then 4.02 would become 4, not 5.

Reference Spec:

​https://tools.ietf.org/html/draft-pantos-http-live-streaming-22#page-49

4.3.3.1. EXT-X-TARGETDURATION

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; longer segments can trigger playback stalls
or other errors.

In other words, we take maximum chunk duration (4.02), round to nearest integer (4). Then EXT-X-TARGETDURATION must be greater or equal to this 4.

Technically, 5 also matches the specification in this case (4 <= 5) but 4 is more precise AND more logical having in mind -hls_time 4 given.

So my proposal matches the specification -- we should round maximum chunk duration to nearest integer, not ceil it.

[Steven Liu]

Replying to tonn81:

Replying to stevenliu:

ffmpeg should generate chunks not longer than given with --hls_time. For --hls_time 4 it generated 4.02 chunk instead of 4.0 or less.

Write a full frame is better than half frame. for example, the frame duration is 40, the frame sequence is: IPPIPPIPP, if the hls_time is set to 100ms. Do you mean it should split IP and half of next P and leave the other half to the next segment? Or leave the full P frame to the next segment start frame to make sure the hls_time 0.100?

I cannot tell here. I just see that chunk duration is longer than requested.

It's is longer but it is the segment logic need.
yes , if you want to fix it, and you need me to fix it, i can sent a patch to you , but i cannot applied it into ffmpeg hlsenc now, because it maybe will make some new bugs, one of the bugs is the describe above.

2. ffmpeg converts chunk duration into EXT-X-TARGETDURATION incorrectly

According to official HLS validator (mediastreamvalidator), there could be 25% difference between actual chunk size and value. That means we should not ceil the value but rather round to nearest integer (so that delta between maximum chunk duration and TARGETDURATION is minimal). Then 4.02 would become 4, not 5.

Reference Spec:

​https://tools.ietf.org/html/draft-pantos-http-live-streaming-22#page-49

4.3.3.1. EXT-X-TARGETDURATION

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; longer segments can trigger playback stalls
or other errors.

In other words, we take maximum chunk duration (4.02), round to nearest integer (4). Then EXT-X-TARGETDURATION should be greater or equal to this 4.

Technically, 5 also matches the specification in this case (4 <= 5) but 4 is more precise AND more logical having in mind -hls_time 4 given.

So my proposal matches the specification -- we should round maximum chunk duration to nearest integer, not ceil it.

The specification said, the target duration should greater than 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;) it said *MUST*.

[Artem Skoretskiy]

Replying to stevenliu:

I cannot tell here. I just see that chunk duration is longer than requested.

It's is longer but it is the segment logic need.
yes , if you want to fix it, and you need me to fix it, i can sent a patch to you , but i cannot applied it into ffmpeg hlsenc now, because it maybe will make some new bugs, one of the bugs is the describe above.

We could keep it as-is if we fix second part of it.

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; longer segments can trigger playback stalls
or other errors.

In other words, we take maximum chunk duration (4.02), round to nearest integer (4). Then EXT-X-TARGETDURATION should be greater or equal to this 4.

Technically, 5 also matches the specification in this case (4 <= 5) but 4 is more precise AND more logical having in mind -hls_time 4 given.

So my proposal matches the specification -- we should round maximum chunk duration to nearest integer, not ceil it.

The specification said, the target duration should greater than 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;) it said *MUST*.

Yeah, MUST not should. But not greater but greater or equal to rounded value.

I.e. round(max{chunk_duration}) <= target_duration

I have provided an example of logic that follows the specification.

[Artem Skoretskiy]

Or, all {round(chunk_duration) <= target_duration}

[Steven Liu]

Replying to tonn81:

Replying to stevenliu:

I cannot tell here. I just see that chunk duration is longer than requested.

It's is longer but it is the segment logic need.
yes , if you want to fix it, and you need me to fix it, i can sent a patch to you , but i cannot applied it into ffmpeg hlsenc now, because it maybe will make some new bugs, one of the bugs is the describe above.

We could keep it as-is if we fix second part of it.

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; longer segments can trigger playback stalls
or other errors.

In other words, we take maximum chunk duration (4.02), round to nearest integer (4). Then EXT-X-TARGETDURATION should be greater or equal to this 4.

Technically, 5 also matches the specification in this case (4 <= 5) but 4 is more precise AND more logical having in mind -hls_time 4 given.

So my proposal matches the specification -- we should round maximum chunk duration to nearest integer, not ceil it.

The specification said, the target duration should greater than 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;) it said *MUST*.

Yeah, MUST not should. But not greater but greater or equal to rounded value.

I.e. round(max{chunk_duration}) <= target_duration

I have provided an example of logic that follows the specification.

now, the chunk_duration is LESS or EQU than target_duration.
if the chunk_duration is LARGER than target_duration, you can show me, let me fix it.

[Steven Liu]

Replying to tonn81:

Or, all {round(chunk_duration) <= target_duration}

If you mean the hls_time is the target duration, i think you are wrong,
the m3u8 should make segment complete(Full frames and full packets, not a part of frame ot a part of packet);
for example, GOP Size if 5s, then you set hls_time to 3s, there will have some problem, because the segment is split at keyframe and full frame, so the hls_time you set is a reference for segment spliter

[Artem Skoretskiy]

OK, let me be more specific.

Specification says this rule must be followed:

all {round(chunk_duration) <= EXT_X_TARGETDURATION}

In current implementation, we have:

EXT_X_TARGETDURATION = ceil(max{chunk_duration})

It gives result that matches the specification (part about less) but not precise.

I suggest to have:

EXT_X_TARGETDURATION = round(max{chunk_duration})

It gives result that matches the specification (part about equal) and precise.

Also, that aligns with the value that user provided as input to ffmpeg (-hls_time) and allows to ignore small difference that HLS segmenter generates (e.g. 4.02 instead of 4.0).

Current implementation:

    for (en = hls->segments; en; en = en->next) {
        if (target_duration <= en->duration)
            # that would actually ceil
            target_duration = get_int_from_double(en->duration);
    }

Suggested implementation (pseudo-code):

    for (en = hls->segments; en; en = en->next) {
        if (target_duration < en->duration)
            # if chunk duration is bigger than target duration -- round it and use it,
            # it could be also the same value as target_duration if rounded to lower value
            target_duration = (int)round(en->duration);
    }

[Steven Liu]

Replying to tonn81:

OK, let me be more specific.

Specification says this rule must be followed:

all {round(chunk_duration) <= EXT_X_TARGETDURATION}

In current implementation, we have:

EXT_X_TARGETDURATION = ceil(max{chunk_duration})

It gives result that matches the specification (part about less) but not precise.

I suggest to have:

EXT_X_TARGETDURATION = round(max{chunk_duration})

It gives result that matches the specification (part about equal) and precise.

Also, that aligns with the value that user provided as input to ffmpeg (-hls_time) and allows to ignore small difference that HLS segmenter generates (e.g. 4.02 instead of 4.0).

Current implementation:

    for (en = hls->segments; en; en = en->next) {
        if (target_duration <= en->duration)
            # that would actually ceil
            target_duration = get_int_from_double(en->duration);
    }

The old code is use ceil, so implement a new API to fix the ceil problem,
So that is not ceil:

static int get_int_from_double(double val)
{

return (int)((val - (int)val) >= 0.001) ? (int)(val + 1) : (int)val;

}

or can you reproduce the bug of the EXT_X_TARGETDURATION *LESS* than en->duration ?

Suggested implementation (pseudo-code):

    for (en = hls->segments; en; en = en->next) {
        if (target_duration < en->duration)
            # if chunk duration is bigger than target duration -- round it and use it,
            # it could be also the same value as target_duration if rounded to lower value
            target_duration = (int)round(en->duration);
    }

[Artem Skoretskiy]

Replying to stevenliu:

That is not ceil:

static int get_int_from_double(double val)
{
    return (int)((val - (int)val) >= 0.001) ? (int)(val + 1) : (int)val;
}

If you check the code, it is ceiling when float part of value is bigger than 0.001.

If you run it:

get_int_from_double(4.000) # 4
get_int_from_double(4.001) # 4
get_int_from_double(4.002) # 5
get_int_from_double(4.003) # 5
get_int_from_double(4.02) # 5

For anything bigger than X.001 it is ceiling:

4.0 => 4
4.1 => 5
4.2 => 5
4.3 => 5
4.4 => 5
4.5 => 5
4.6 => 5
4.7 => 5
4.8 => 5
4.9 => 5
5.0 => 5
2.1 => 3

or can you reproduce the bug of the EXT_X_TARGETDURATION *LESS* than en->duration ?

Since get_int_from_double is making ceiling the most of the time, EXT_X_TARGETDURATION would be always bigger than en->duration

I propose one-line change that gives better result (more precise and the one expected by user).

[Steven Liu]

Replying to tonn81:

Replying to stevenliu:

That is not ceil:

static int get_int_from_double(double val)
{
    return (int)((val - (int)val) >= 0.001) ? (int)(val + 1) : (int)val;
}

If you check the code, it is ceiling when float part of value is bigger than 0.001.

If you run it:

get_int_from_double(4.000) # 4
get_int_from_double(4.001) # 4
get_int_from_double(4.002) # 5
get_int_from_double(4.003) # 5
get_int_from_double(4.02) # 5

For anything bigger than X.001 it is ceiling:

0.001 = 1ms
if the frame duration is LESS or EQU than 1ms, maybe the fps = 999? can you reproduce the bug on your compute? if you can, i can modify the val from 0.001 to 0.000001.

4.0 => 4
4.1 => 5
4.2 => 5
4.3 => 5
4.4 => 5
4.5 => 5
4.6 => 5
4.7 => 5
4.8 => 5
4.9 => 5
5.0 => 5
2.1 => 3

or can you reproduce the bug of the EXT_X_TARGETDURATION *LESS* than en->duration ?

Since get_int_from_double is making ceiling the most of the time, EXT_X_TARGETDURATION would be always bigger than en->duration

I propose one-line change that gives better result (more precise and the one expected by user).

[Steven Liu]

Replying to tonn81:

Replying to stevenliu:

That is not ceil:

static int get_int_from_double(double val)
{
    return (int)((val - (int)val) >= 0.001) ? (int)(val + 1) : (int)val;
}

If you check the code, it is ceiling when float part of value is bigger than 0.001.

If you run it:

get_int_from_double(4.000) # 4
get_int_from_double(4.001) # 4
get_int_from_double(4.002) # 5
get_int_from_double(4.003) # 5
get_int_from_double(4.02) # 5

For anything bigger than X.001 it is ceiling:

4.0 => 4
4.1 => 5
4.2 => 5
4.3 => 5
4.4 => 5
4.5 => 5
4.6 => 5
4.7 => 5
4.8 => 5
4.9 => 5
5.0 => 5
2.1 => 3

or can you reproduce the bug of the EXT_X_TARGETDURATION *LESS* than en->duration ?

Since get_int_from_double is making ceiling the most of the time, EXT_X_TARGETDURATION would be always bigger than en->duration

I propose one-line change that gives better result (more precise and the one expected by user).

Hi tonn81,

I will modify the val, from 0.001 to 0.000001, that is 1 microsecound, transform to frames per second is 999999fps.

(Meet your wishes)

[Artem Skoretskiy]

@stevenliu, do you see the problem I have and the way I see to fix the issue?

It seems you don't see the whole picture and falling to technical details ignoring all the arguments I provided.

Is it trolling?

The problem is that (because of HLS segmenter generating chunks slightly longer than requested) EXT_X_TARGETDURATION value is too high.

That makes EXT_X_TARGETDURATION match the specification but not precise and not matching users needs.

Now if chunk duration is more than 4.001 (or 4.0000001 after your change), it would result in EXT_X_TARGETDURATION = 5.

I propose if chunk duration is < 4.5 to have EXT_X_TARGETDURATION = 4 and if chunk duration is >= 4.5 to have EXT_X_TARGETDURATION = 5.

That would require exactly to change get_int_from_double(en->duration) to (int)round(en->duration).

[Artem Skoretskiy]

Created pull request here ​https://github.com/FFmpeg/FFmpeg/pull/265

[Steven Liu]

Replying to tonn81:

@stevenliu, do you see the problem I have and the way I see to fix the issue?

It seems you don't see the whole picture and falling to technical details ignoring all the arguments I provided.

Is it trolling?

The problem is that (because of HLS segmenter generating chunks slightly longer than requested) EXT_X_TARGETDURATION value is too high.

That makes EXT_X_TARGETDURATION match the specification but not precise and not matching users needs.

Now if chunk duration is more than 4.001 (or 4.0000001 after your change), it would result in EXT_X_TARGETDURATION = 5.

I propose if chunk duration is < 4.5 to have EXT_X_TARGETDURATION = 4 and if chunk duration is >= 4.5 to have EXT_X_TARGETDURATION = 5.

That would require exactly to change get_int_from_double(en->duration) to (int)round(en->duration).

i have read your mind and all, let me make sure what I understand of you.

You mean segment duration is x, if x = 4.400 then TARGETDURATION = 4 , Is that your mean?

[Artem Skoretskiy]

Replying to stevenliu:

i have read your mind and all, let me make sure what I understand of you.

You mean segment duration is x, if x = 4.400 then TARGETDURATION = 4 , Is that your mean?

Right. If segment duration is 4.400, then TARGETDURATION should be 4 (not 5 as it is now).

[Timo R.]

I don't get the whole argument here. The spec clearly says it has to be greater or equal. So rounding it up is perfectly legal and might even safeguard against some poor player implementations.

[Artem Skoretskiy]

Right, it is legal and "safe".

But it is less precise and different than user expects.

Just for a second, if I run:

ffmpeg ... -hls_time 4

I expect:

#EXT-X-TARGETDURATION:4

But I get 5!

Just because HLS segmenter generated one segment with 4.02 duration instead of 4.00 or 3.98 and "safeguard" M3U8 generator decided to ceil 4.02 to 5.

[Steven Liu]

Replying to tonn81:

Right, it is legal and "safe".

But it is less precise and different than user expects.

Just for a second, if I run:

ffmpeg ... -hls_time 4

I expect:

#EXT-X-TARGETDURATION:4

But I get 5!

Just because HLS segmenter generated one segment with 4.02 duration instead of 4.00 or 3.98 and "safeguard" M3U8 generator decided to ceil 4.02 to 5.

I have get your mean, but I won't implement it, if you want fix , I can send you a patch but won't applied in ffmpeg hlsenc.

[Steven Liu]

The feature required is not in standard specification description, and the hls_time just a reference fragment split time point.

[Stephen Hoogendijk]

According to the spec:

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; longer segments can trigger playback stalls
or other errors. It applies to the entire Playlist file. Its format
is:

#EXT-X-TARGETDURATION:<s>

​https://tools.ietf.org/html/rfc8216#section-4.3.3.1

The way that the rounding is being done is invalid:
​https://github.com/FFmpeg/FFmpeg/blob/23678462c0a316772d112ca363a4ffdf7d4b4236/libavformat/hlsenc.c#L1482

It results in invalid playlists:

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:8
#EXT-X-MEDIA-SEQUENCE:0
#EXTINF:8.250000,
0000.ts
#EXTINF:7.227000,
0001.ts
#EXT-X-ENDLIST

This playlist (while playable in ffplay, but not in most browser-based players) is invalid according to the specification.

[Hendrik]

If you carefully read the spec excerpt you posted yourself, I don't see how its invalid. Rounding 8.25 to the nearest integer is 8, which is "less than or equal" to targetduration 8. Rounding 7.227 is 7, which is also "less than or equal" to targetduration.

[Steven Liu]

Replying to thecodeassassin:

According to the spec:

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; longer segments can trigger playback stalls
or other errors. It applies to the entire Playlist file. Its format
is:

#EXT-X-TARGETDURATION:<s>

​https://tools.ietf.org/html/rfc8216#section-4.3.3.1

The way that the rounding is being done is invalid:
​https://github.com/FFmpeg/FFmpeg/blob/23678462c0a316772d112ca363a4ffdf7d4b4236/libavformat/hlsenc.c#L1482

It results in invalid playlists:

#EXTM3U
#EXT-X-VERSION:3
#EXT-X-TARGETDURATION:8
#EXT-X-MEDIA-SEQUENCE:0
#EXTINF:8.250000,
0000.ts
#EXTINF:7.227000,
0001.ts
#EXT-X-ENDLIST

This playlist (while playable in ffplay, but not in most browser-based players) is invalid according to the specification.

reference this link:
​https://trac.ffmpeg.org/ticket/6915

And i have checked this problem from Pantos:

Roger Pantos  于2017年12月22日周五 上午11:11写道：



 > On Dec 21, 2017, at 3:36 PM, Steven Liu  wrote:
 >
 > 2017-12-22 7:22 GMT+08:00 Roger Pantos :
 >> If the longest EXTINF is 1.02 then you should set EXT-X-TARGETDURATION to 1.
 > Ok, i will modify that use lrint(EXTINF) to set the EXT-X-TARGETDURATION.

 I think you probably want something like this:

  EXT-X-TARGETDURATION_value = MAX(EXT-X-TARGETDURATION_value, lrint(EXTINF_value));


 Roger.

[Steven Liu]

This playlist (while playable in ffplay, but not in most browser-based players) is invalid according to the specification.

Which browser? Can you provide me to reproduce it?