PR #24884 opened by kaecyra URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24884 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24884.patch
# Summary of changes Fixes #24883. In DASH mode movenc rewrites the pts of the first sample of each fragment to the end_pts of the previous fragment, so the sidx timeline stays contiguous. With open GOP content the first sample in decoding order is a keyframe followed by leading pictures with lower pts, so the keyframe gets the pts of a leading picture. Removing the rewrite exposes an additional underlying problem. When the dts of the first packet of a fragment is earlier than the end of the previous fragment, check_pkt() corrects the dts and drops the pts, and the packet falls back to pts = dts. This happens when the last packet of a fragment declares a longer duration than the actual gap to the next packet, for example with Matroska timestamps rounded to milliseconds. The first commit makes check_pkt() move the pts by the same amount as the dts, so the packet keeps its composition offset. The second commit computes the sidx earliest presentation time and duration from the lowest pts in the fragment and stops rewriting the pts of the first sample. In this order each commit is correct on its own. Smooth Streaming output still uses the rewrite, because tfxd and tfrf describe a fragment by its first sample. Each commit adds a case to api-movenc-test, and no existing FATE reference changes. FATE passes after each commit. On a 65 second open GOP HEVC clip, no segment has a duplicate pts and every keyframe is within 1 ms of its source pts, where 7.1.5 moved 3 keyframes by 0.17 to 0.29 seconds. The 1 ms comes from the source: its packets declare 42 ms durations for 41.708 ms frames, and a fragment cannot start before the durations already written for the previous one. **I diagnosed the bug and wrote this patch with an AI assistant (Claude).** I reviewed each change and verified it as described above. >From a26e7b352f2cccb2077117ad8542e0718ff64041 Mon Sep 17 00:00:00 2001 From: Tim Gunter <[email protected]> Date: Mon, 5 Oct 2026 03:11:35 -0400 Subject: [PATCH 1/2] avformat/movenc: keep the composition offset when correcting a backwards dts When check_pkt() has to move a dts that went backwards, it also drops the pts, and ff_mov_write_packet() then falls back to pts = dts. At a fragment boundary this happens whenever the last packet of the previous fragment has an overstated duration, for example with millisecond rounded Matroska timestamps, and the first sample of the new fragment loses its composition offset. Shift the pts by the same amount as the dts instead. Signed-off-by: Tim Gunter <[email protected]> --- libavformat/movenc.c | 6 +++++- tests/api/api-movenc-test.c | 12 ++++++++++++ tests/ref/fate/api-movenc | 6 ++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 6492f4ffb3..bfb47e9e91 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -7012,8 +7012,12 @@ static int check_pkt(AVFormatContext *s, MOVTrack *trk, AVPacket *pkt) av_log(s, AV_LOG_WARNING, "Packet duration: %"PRId64" / dts: %"PRId64" in stream %d is out of range\n", duration, pkt->dts, pkt->stream_index); + if (pkt->dts < ref && pkt->pts != AV_NOPTS_VALUE) { + pkt->pts += ref + 1 - pkt->dts; + } else { + pkt->pts = AV_NOPTS_VALUE; + } pkt->dts = ref + 1; - pkt->pts = AV_NOPTS_VALUE; } if (pkt->duration < 0 || pkt->duration > INT_MAX) { diff --git a/tests/api/api-movenc-test.c b/tests/api/api-movenc-test.c index 39494748fd..e6eb41d47a 100644 --- a/tests/api/api-movenc-test.c +++ b/tests/api/api-movenc-test.c @@ -843,6 +843,18 @@ int main(int argc, char **argv) finish(); close_out(); + // Overstated duration of the last packet before a fragment boundary. + init_out("empty-moov-overlong-duration"); + av_dict_set(&opts, "movflags", "+frag_custom+empty_moov", 0); + init(1, 0); + fake_pkt_duration = 2 * duration; + mux_gops(1); + fake_pkt_duration = 0; + av_write_frame(ctx, NULL); + mux_gops(1); + finish(); + close_out(); + av_free(md5); av_packet_free(&pkt); diff --git a/tests/ref/fate/api-movenc b/tests/ref/fate/api-movenc index 20996b8910..04ea5818f1 100644 --- a/tests/ref/fate/api-movenc +++ b/tests/ref/fate/api-movenc @@ -163,3 +163,9 @@ write_data len 70, time 2000000, type boundary atom emsg write_data len 1840, time 2000000, type sync atom moof write_data len 148, time nopts, type trailer atom - 43f9d447aa759ec494e20d9c8d149488 5111 emsg +write_data len 36, time nopts, type header atom ftyp +write_data len 1123, time nopts, type header atom - +write_data len 1188, time 0, type sync atom moof +write_data len 1028, time 1033333, type sync atom moof +write_data len 148, time nopts, type trailer atom - +08fcab873f7ac255c3aa46248b2bc7c9 3523 empty-moov-overlong-duration -- 2.52.0 >From 68e5f770b67e2e477ab3c5947a846c7dba99170f Mon Sep 17 00:00:00 2001 From: Tim Gunter <[email protected]> Date: Mon, 5 Oct 2026 03:11:36 -0400 Subject: [PATCH 2/2] avformat/movenc: use the earliest pts of a fragment in sidx In DASH mode the pts of the first sample of every fragment is rewritten to the end_pts of the previous fragment, to keep the sidx timeline contiguous. This assumes that the first sample in decoding order is also the first one presented. With open GOP content, such as HEVC with CRA keyframes followed by RASL pictures, the keyframe ends up with the pts of a leading picture, and players drop or skip the following frames. Compute the sidx earliest presentation time and the fragment duration from the lowest pts in the fragment instead, and leave the sample timestamps alone. The rewrite is kept for Smooth Streaming, where tfxd and tfrf still describe the fragment by its first sample. Signed-off-by: Tim Gunter <[email protected]> --- libavformat/movenc.c | 24 +++++++++++++++--------- tests/api/api-movenc-test.c | 20 ++++++++++++++++++++ tests/ref/fate/api-movenc | 6 ++++++ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/libavformat/movenc.c b/libavformat/movenc.c index bfb47e9e91..5bb7672613 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1329,6 +1329,16 @@ static int get_cluster_duration(MOVTrack *track, int cluster_idx) return next_dts; } +static int64_t get_earliest_pts(const MOVTrack *track) +{ + int64_t pts = track->cluster[0].dts + track->cluster[0].cts; + + for (int i = 1; i < track->entry; i++) + pts = FFMIN(pts, track->cluster[i].dts + track->cluster[i].cts); + + return pts; +} + static int get_samples_per_packet(MOVTrack *track) { int i, first_duration; @@ -5916,8 +5926,7 @@ static int mov_add_tfra_entries(AVIOContext *pb, MOVMuxContext *mov, int tracks, // Try to recreate the original pts for the first packet // from the fields we have stored info->time = track->cluster[0].dts + track->cluster[0].cts; - info->duration = track->end_pts - - (track->cluster[0].dts + track->cluster[0].cts); + info->duration = track->end_pts - get_earliest_pts(track); // If the pts is less than zero, we will have trimmed // away parts of the media track using an edit list, // and the corresponding start presentation time is zero. @@ -6034,9 +6043,8 @@ static int mov_write_sidx_tag(AVIOContext *pb, if (track->entry) { entries = 1; - presentation_time = track->cluster[0].dts + track->cluster[0].cts; - duration = track->end_pts - - (track->cluster[0].dts + track->cluster[0].cts); + presentation_time = get_earliest_pts(track); + duration = track->end_pts - presentation_time; starts_with_SAP = track->cluster[0].flags & MOV_SYNC_SAMPLE; // pts<0 should be cut away using edts @@ -7400,13 +7408,11 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt) * of this packet to be what the previous packets duration implies. */ trk->cluster[trk->entry].dts = trk->start_dts + trk->track_duration; /* We also may have written the pts and the corresponding duration - * in sidx/tfrf/tfxd tags; make sure the sidx pts and duration match up with + * in tfrf/tfxd tags; make sure the pts and duration match up with * the next fragment. This means the cts of the first sample must * be the same in all fragments, unless end_pts was updated by * the packet causing the fragment to be written. */ - if ((mov->flags & FF_MOV_FLAG_DASH && - !(mov->flags & (FF_MOV_FLAG_GLOBAL_SIDX | FF_MOV_FLAG_SKIP_SIDX))) || - mov->mode == MODE_ISM) + if (mov->mode == MODE_ISM) pkt->pts = pkt->dts + trk->end_pts - trk->cluster[trk->entry].dts; } else { /* New fragment, but discontinuous from previous fragments. diff --git a/tests/api/api-movenc-test.c b/tests/api/api-movenc-test.c index e6eb41d47a..33295b2795 100644 --- a/tests/api/api-movenc-test.c +++ b/tests/api/api-movenc-test.c @@ -74,6 +74,7 @@ static int clear_duration; static int force_iobuf_size; static int do_interleave; static int fake_pkt_duration; +static int open_gop; static int num_warnings; @@ -283,6 +284,11 @@ static void mux_frames(int n, int c) last_picture = AV_PICTURE_TYPE_I; pkt->pts = pkt->dts + duration; video_dts = pkt->pts; + if (open_gop && frames) { + last_picture = AV_PICTURE_TYPE_P; + next_p_pts = pkt->pts = pkt->dts + 2 * duration; + video_dts = pkt->dts + duration; + } } else { if (last_picture == AV_PICTURE_TYPE_P) { last_picture = AV_PICTURE_TYPE_B; @@ -855,6 +861,20 @@ int main(int argc, char **argv) finish(); close_out(); + // Open GOP: each keyframe after the first is followed by a leading + // frame with a lower pts. + open_gop = 1; + av_dict_set(&opts, "movflags", "+frag_custom+empty_moov+dash", 0); + init(1, 0); + mux_gops(1); + av_write_frame(ctx, NULL); + init_out("empty-moov-second-frag-open-gop"); + mux_gops(1); + av_write_frame(ctx, NULL); + close_out(); + finish(); + open_gop = 0; + av_free(md5); av_packet_free(&pkt); diff --git a/tests/ref/fate/api-movenc b/tests/ref/fate/api-movenc index 04ea5818f1..496ab604fd 100644 --- a/tests/ref/fate/api-movenc +++ b/tests/ref/fate/api-movenc @@ -169,3 +169,9 @@ write_data len 1188, time 0, type sync atom moof write_data len 1028, time 1033333, type sync atom moof write_data len 148, time nopts, type trailer atom - 08fcab873f7ac255c3aa46248b2bc7c9 3523 empty-moov-overlong-duration +write_data len 28, time nopts, type header atom - +write_data len 1123, time nopts, type header atom - +write_data len 1156, time 0, type sync atom sidx +write_data len 996, time 1000000, type sync atom sidx +0e59030422e18fd8f4a51c019222337a 996 empty-moov-second-frag-open-gop +write_data len 148, time nopts, type trailer atom - -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
