PR #23740 opened by michaelni URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23740 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23740.patch
This PR fixes assertion failure, missing eviction f the 2nd field in h264 (happens in fate) unbounded collection of frames (after the assert is fixed) The patchset is written with AI and i went back and forth with it over serveral iterations of several hours, this is the first iteration that from a high level view isnt smelling wrong, or maybe iam just tired to do another iteraion >From 3100b78eec73673168d76a39d783dd08710c8323 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Sun, 5 Jul 2026 01:38:19 +0200 Subject: [PATCH 1/3] avcodec/bsf/dts2pts: chain same-gop nodes instead of collecting into a fixed array hevc_queue_frame() collected all tree nodes of the current gop into a fixed nodes[HEVC_MAX_DPB_SIZE * 2] array and asserted the count stayed within it. For a crafted HEVC stream the tree can hold slightly more same-gop nodes than that estimate (observed 34 vs 32), tripping the av_assert0 and aborting. Link the matching nodes into a list through a next pointer in the node instead; this has no size limit and needs neither extra passes nor allocation. Fixes: assertion failure Fixes: 519466146/clusterfuzz-testcase-minimized-ffmpeg_BSF_DTS2PTS_fuzzer-5238235193475072 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Co-Authored-By: Fable-5 --- libavcodec/bsf/dts2pts.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/libavcodec/bsf/dts2pts.c b/libavcodec/bsf/dts2pts.c index b620c3f837..de44eaaedd 100644 --- a/libavcodec/bsf/dts2pts.c +++ b/libavcodec/bsf/dts2pts.c @@ -46,6 +46,7 @@ typedef struct DTS2PTSNode { int64_t duration; int poc; int gop; + struct DTS2PTSNode *next; // valid only during same-gop re-keying } DTS2PTSNode; typedef struct DTS2PTSFrame { @@ -415,9 +416,7 @@ static int hevc_init_nb_frame(AVBSFContext *ctx, int poc) typedef struct DTS2PTSCollect { int gop; - DTS2PTSNode **out; - int count; - int max; + DTS2PTSNode *head, *tail; } DTS2PTSCollect; static int collect_same_gop(void *opaque, void *elem) @@ -425,9 +424,12 @@ static int collect_same_gop(void *opaque, void *elem) DTS2PTSCollect *c = opaque; DTS2PTSNode *node = elem; if (node->gop == c->gop) { - if (c->count < c->max) - c->out[c->count] = node; - c->count++; + if (c->tail) + c->tail->next = node; + else + c->head = node; + c->tail = node; + node->next = NULL; } return 0; } @@ -454,22 +456,26 @@ static int hevc_queue_frame(AVBSFContext *ctx, AVPacket *pkt, int poc, bool *que if (poc < s->nb_frame && hevc->gop == s->gop) { int dec = s->nb_frame - poc; - DTS2PTSNode *nodes[HEVC_MAX_DPB_SIZE * 2]; - DTS2PTSCollect c = { s->gop, nodes, 0, FF_ARRAY_ELEMS(nodes) }; + DTS2PTSCollect c = { s->gop, NULL, NULL }; s->nb_frame -= dec; + // Crafted streams can exceed any DPB-based estimate of the node count, + // so chain the matching nodes through their next pointers instead of + // collecting them into a fixed size array. The chain is in ascending + // poc order; processing it in this order keeps the new keys collision + // free as any potential collision partner is re-keyed first. av_tree_enumerate(s->root, &c, NULL, collect_same_gop); - av_assert0(c.count <= c.max); - for (int i = 0; i < c.count; i++) { + while (c.head) { struct AVTreeNode *tnode = NULL; - DTS2PTSNode *r; - av_tree_insert(&s->root, nodes[i], cmp_insert, &tnode); - nodes[i]->poc -= dec; - r = av_tree_insert(&s->root, nodes[i], cmp_insert, &tnode); - if (r && r != nodes[i]) { - *r = *nodes[i]; - av_refstruct_unref(&nodes[i]); + DTS2PTSNode *node = c.head, *r; + c.head = node->next; + av_tree_insert(&s->root, node, cmp_insert, &tnode); + node->poc -= dec; + r = av_tree_insert(&s->root, node, cmp_insert, &tnode); + if (r && r != node) { + *r = *node; + av_refstruct_unref(&node); av_free(tnode); } } -- 2.52.0 >From 70993bb216d03f885c6fb162bd209c7b49f19515 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Wed, 8 Jul 2026 17:24:49 +0200 Subject: [PATCH 2/3] avcodec/bsf/dts2pts: fix removal of 2nd field nodes alloc_and_insert_node() inserts the nodes for 2nd fields with duration / poc_diff added to the timestamp, but the removal loop compared all nodes of a frame against the unadjusted first timestamp, so 2nd field nodes never matched and stayed in the tree until close. Advance the compared timestamp the same way the insertion does. Fixes: tree nodes leaking on every field coded frame Co-Authored-By: Fable-5 --- libavcodec/bsf/dts2pts.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libavcodec/bsf/dts2pts.c b/libavcodec/bsf/dts2pts.c index de44eaaedd..b0b6b0ec23 100644 --- a/libavcodec/bsf/dts2pts.c +++ b/libavcodec/bsf/dts2pts.c @@ -680,10 +680,14 @@ static int dts2pts_filter(AVBSFContext *ctx, AVPacket *out) if (!s->eof) { // Remove the found entry from the tree DTS2PTSFrame dup = (DTS2PTSFrame) { NULL, frame.poc + 1, frame.poc_diff, frame.gop }; + int64_t dts = out->pts; for (; dup.poc_diff > 0; dup.poc++, dup.poc_diff--) { struct AVTreeNode *node = NULL; - if (!poc_node || poc_node->dts != out->pts) + if (!poc_node || poc_node->dts != dts) continue; + // 2nd field nodes were inserted with this offset added + if (dts != AV_NOPTS_VALUE) + dts += poc_node->duration / frame.poc_diff; av_tree_insert(&s->root, poc_node, cmp_insert, &node); av_refstruct_unref(&poc_node); av_free(node); -- 2.52.0 >From eac107f80502e368cf4f4a303de5e2906ab7b141 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Wed, 8 Jul 2026 16:14:48 +0200 Subject: [PATCH 3/3] avcodec/bsf/dts2pts: evict unconsumable nodes from the poc tree Each pending packet consumes up to poc_diff tree nodes when it is output, so the packets in the FIFO can consume at most nb_pending nodes in total. Frames whose tree lookup misses on output leave their nodes behind, and damaged or crafted streams can make that happen indefinitely, growing the tree without limit. Track the node count and insertion order, keep the leftovers of up to MAX_DAMAGED_FRAMES frames and beyond that evict the nodes unconsumed the longest. Timestamps of valid frames are unaffected unless more frames than that are damaged. No eviction is done at EOF, where nodes are deliberately kept to regenerate timestamps from. Of all h264/hevc conformance samples only MR3_TANDBERG_B.264 triggers evictions, and no sample changes output. Fixes: unbounded memory growth with damaged streams Co-Authored-By: Fable-5 --- libavcodec/bsf/dts2pts.c | 51 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/libavcodec/bsf/dts2pts.c b/libavcodec/bsf/dts2pts.c index b0b6b0ec23..03e7c39983 100644 --- a/libavcodec/bsf/dts2pts.c +++ b/libavcodec/bsf/dts2pts.c @@ -41,11 +41,17 @@ #include "libavcodec/h264_ps.h" #include "libavcodec/hevc/ps.h" +// Damaged frames leave their up to 2 timestamp nodes behind unconsumed. +// This many damaged frames are tolerated before the oldest leftovers are +// evicted; no timestamp of a valid frame is lost below this. +#define MAX_DAMAGED_FRAMES 32 + typedef struct DTS2PTSNode { int64_t dts; int64_t duration; int poc; int gop; + int64_t serial; // insertion order, evicting the stalest node first struct DTS2PTSNode *next; // valid only during same-gop re-keying } DTS2PTSNode; @@ -90,6 +96,9 @@ typedef struct DTS2PTSContext { DTS2PTSHEVCContext hevc; } u; + int nb_nodes; + int nb_pending; + int64_t serial; int nb_frame; int gop; int eof; @@ -129,11 +138,21 @@ static int free_node(void *opaque, void *elem) return 0; } +static int find_stalest(void *opaque, void *elem) +{ + DTS2PTSNode **stalest = opaque; + DTS2PTSNode *node = elem; + if (!*stalest || node->serial < (*stalest)->serial) + *stalest = node; + return 0; +} + // Shared functions static int alloc_and_insert_node(AVBSFContext *ctx, int64_t ts, int64_t duration, int poc, int poc_diff, int gop) { DTS2PTSContext *s = ctx->priv_data; + for (int i = 0; i < poc_diff; i++) { struct AVTreeNode *node = av_tree_node_alloc(); DTS2PTSNode *poc_node, *ret; @@ -146,13 +165,14 @@ static int alloc_and_insert_node(AVBSFContext *ctx, int64_t ts, int64_t duration } if (i && ts != AV_NOPTS_VALUE) ts += duration / poc_diff; - *poc_node = (DTS2PTSNode) { ts, duration, poc++, gop }; + *poc_node = (DTS2PTSNode) { ts, duration, poc++, gop, s->serial++ }; ret = av_tree_insert(&s->root, poc_node, cmp_insert, &node); if (ret && ret != poc_node) { *ret = *poc_node; av_refstruct_unref(&poc_node); av_free(node); - } + } else + s->nb_nodes++; } return 0; } @@ -230,6 +250,7 @@ static int h264_queue_frame(AVBSFContext *ctx, AVPacket *pkt, int poc, int *queu frame = (DTS2PTSFrame) { pkt, poc, poc_diff, s->gop }; ret = av_fifo_write(s->fifo, &frame, 1); av_assert2(ret >= 0); + s->nb_pending += poc_diff; *queued = 1; return 0; @@ -477,6 +498,7 @@ static int hevc_queue_frame(AVBSFContext *ctx, AVPacket *pkt, int poc, bool *que *r = *node; av_refstruct_unref(&node); av_free(tnode); + s->nb_nodes--; } } } @@ -498,6 +520,7 @@ static int hevc_queue_frame(AVBSFContext *ctx, AVPacket *pkt, int poc, bool *que ret = av_fifo_write(s->fifo, &frame, 1); if (ret < 0) return ret; + s->nb_pending += frame.poc_diff; *queued = true; @@ -665,6 +688,7 @@ static int dts2pts_filter(AVBSFContext *ctx, AVPacket *out) // Fetch a packet from the FIFO ret = av_fifo_read(s->fifo, &frame, 1); av_assert2(ret >= 0); + s->nb_pending -= frame.poc_diff; av_packet_move_ref(out, frame.pkt); av_packet_free(&frame.pkt); @@ -691,6 +715,7 @@ static int dts2pts_filter(AVBSFContext *ctx, AVPacket *out) av_tree_insert(&s->root, poc_node, cmp_insert, &node); av_refstruct_unref(&poc_node); av_free(node); + s->nb_nodes--; poc_node = av_tree_find(s->root, &dup, cmp_find, NULL); } } @@ -715,6 +740,24 @@ static int dts2pts_filter(AVBSFContext *ctx, AVPacket *out) av_log(ctx, AV_LOG_WARNING, "No timestamp for POC %d in tree\n", frame.poc); } else av_log(ctx, AV_LOG_WARNING, "No timestamp for POC %d in tree\n", frame.poc); + + // The pending packets consume nb_pending nodes; frames whose lookup above + // missed leave nodes behind which nothing consumes anymore. Keep the + // leftovers of up to MAX_DAMAGED_FRAMES frames, then evict the nodes + // unconsumed the longest. + // At EOF nodes are deliberately kept to regenerate timestamps from. + while (!s->eof && s->nb_nodes > s->nb_pending + 2 * MAX_DAMAGED_FRAMES) { + DTS2PTSNode *stale = NULL; + struct AVTreeNode *tnode = NULL; + av_tree_enumerate(s->root, &stale, NULL, find_stalest); + av_log(ctx, AV_LOG_WARNING, "Evicting unconsumed POC %d, GOP %d\n", + stale->poc, stale->gop); + av_tree_insert(&s->root, stale, cmp_insert, &tnode); + av_refstruct_unref(&stale); + av_free(tnode); + s->nb_nodes--; + } + av_log(ctx, AV_LOG_DEBUG, "Returning frame for POC %d, GOP %d, dts %"PRId64", pts %"PRId64"\n", frame.poc, frame.gop, out->dts, out->pts); @@ -736,7 +779,9 @@ static void dts2pts_flush(AVBSFContext *ctx) av_tree_enumerate(s->root, NULL, NULL, free_node); av_tree_destroy(s->root); - s->root = NULL; + s->root = NULL; + s->nb_nodes = 0; + s->nb_pending = 0; ff_cbs_fragment_reset(&s->au); if (s->cbc) -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
