Opened 3 months ago

Closed 2 months ago

#6533 closed defect (wontfix)

Invalid EXT-X-TARGETDURATION in HLS

Reported by: tonn81 Owned by:
Priority: normal Component: avformat
Version: unspecified Keywords: hls
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

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%
  1. 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
  1. Check results:
grep EXT-X-TARGETDURATION file.m3u8
#EXT-X-TARGETDURATION:5

Expected result:

grep EXT-X-TARGETDURATION file.m3u8
#EXT-X-TARGETDURATION:5

Change History (24)

comment:1 follow-up: Changed 3 months ago by 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
  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);
    }

comment:2 in reply to: ↑ 1 Changed 3 months ago by stevenliu

  • Status changed from new to open

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);
    }

comment:3 Changed 3 months ago by stevenliu

  • Resolution set to wontfix
  • Status changed from open to closed

The duration is correct

comment:4 follow-up: Changed 2 months ago by tonn81

  • Resolution wontfix deleted
  • Status changed from closed to reopened

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.

  1. 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.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 2 months ago by stevenliu

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?

  1. 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.

Last edited 2 months ago by stevenliu (previous) (diff)

comment:6 in reply to: ↑ 5 ; follow-up: Changed 2 months ago by 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.

  1. 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.

Last edited 2 months ago by tonn81 (previous) (diff)

comment:7 in reply to: ↑ 6 Changed 2 months ago by stevenliu

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.

  1. 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*.

comment:8 follow-up: Changed 2 months ago by 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.

comment:9 follow-up: Changed 2 months ago by tonn81

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

comment:10 in reply to: ↑ 8 Changed 2 months ago by stevenliu

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.

comment:11 in reply to: ↑ 9 Changed 2 months ago by stevenliu

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

Last edited 2 months ago by stevenliu (previous) (diff)

comment:12 follow-up: Changed 2 months ago by 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);
    }

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);
    }
Last edited 2 months ago by tonn81 (previous) (diff)

comment:13 in reply to: ↑ 12 ; follow-up: Changed 2 months ago by stevenliu

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);
    }
Last edited 2 months ago by stevenliu (previous) (diff)

comment:14 in reply to: ↑ 13 ; follow-ups: Changed 2 months ago by 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).

Last edited 2 months ago by tonn81 (previous) (diff)

comment:15 in reply to: ↑ 14 Changed 2 months ago by stevenliu

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).

comment:16 in reply to: ↑ 14 Changed 2 months ago by stevenliu

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)

comment:17 follow-up: Changed 2 months ago by 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).

comment:18 Changed 2 months ago by tonn81

comment:19 in reply to: ↑ 17 ; follow-up: Changed 2 months ago by stevenliu

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?

comment:20 in reply to: ↑ 19 Changed 2 months ago by tonn81

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).

comment:21 Changed 2 months ago by oromit

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.

comment:22 follow-up: Changed 2 months ago by 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.

comment:23 in reply to: ↑ 22 Changed 2 months ago by stevenliu

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.

comment:24 Changed 2 months ago by stevenliu

  • Resolution set to wontfix
  • Status changed from reopened to closed

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

Note: See TracTickets for help on using tickets.