PR #25370 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25370 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25370.patch
In-band SPS/PPS updates survive `av_bsf_flush()`, so seeking back can prepend parameter sets from later in the stream to the first IDR picture. The valid PPS-update scenario in #25322 produces a different first decoded picture after flushing than with a freshly initialized filter. Clear private state on flush and restore the initial extradata on the next packet, before applying packet-carried new extradata. Deferring restoration lets the filter return allocation errors, and failed restoration clears partial state before a retry. Output codec parameters remain unchanged after initialization. Add `fate-api-h264-mp4toannexb-flush` with 11 cases covering in-band SPS/PPS updates, missing initial parameter sets, changed NAL length sizes, empty and Annex B initial extradata, new extradata after seeking, and repeated flushes. Expected results are stored in `tests/ref/fate`; no external sample is needed for this test. Validation on aarch64: - The valid-stream scenario from #25322 changes from unequal 9,568/9,569-byte outputs to identical 9,569-byte outputs with matching decoded pictures. - The new FATE test fails with the original master filter and passes with this change. - All five existing H.264 mp4toannexb FATE tests pass. - The new API test and valid-stream reproducer pass under ASan/UBSan without reports. The regression was verified at d3aa0cd16f5e against its passing parent 4c4b833abd5b. Fixes #25322. >From 737e5f319355b0235d44efa94fee39351b94bd7c Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Thu, 22 Oct 2026 17:57:20 +0000 Subject: [PATCH] avcodec/bsf/h264_mp4toannexb: reset parameter sets on flush In-band SPS/PPS updates replace the parameter sets parsed at init, but flush only resets the IDR flags. Seeking back can therefore prepend parameter sets from later in the stream to the first IDR picture. Discard the private state on flush and reload the initial extradata on the next packet, before handling any packet extradata. Deferring the reload allows allocation failures to be returned through the filter callback. Clear partially restored state on failure so a retry is safe. Keep par_out unchanged after init. Add an API FATE test covering SPS and PPS updates, absent initial parameter sets, changed NAL length sizes, Annex B and empty initial extradata, new extradata after seeking, and repeated flushes. Fixes #25322. Assisted-by: Fairy --- libavcodec/bsf/h264_mp4toannexb.c | 32 ++- tests/api/Makefile | 1 + tests/api/api-h264-mp4toannexb-flush-test.c | 260 ++++++++++++++++++++ tests/fate/api.mak | 4 + tests/ref/fate/api-h264-mp4toannexb-flush | 11 + 5 files changed, 302 insertions(+), 6 deletions(-) create mode 100644 tests/api/api-h264-mp4toannexb-flush-test.c create mode 100644 tests/ref/fate/api-h264-mp4toannexb-flush diff --git a/libavcodec/bsf/h264_mp4toannexb.c b/libavcodec/bsf/h264_mp4toannexb.c index 216fea92d17..8d9ea5c3e33 100644 --- a/libavcodec/bsf/h264_mp4toannexb.c +++ b/libavcodec/bsf/h264_mp4toannexb.c @@ -44,6 +44,7 @@ typedef struct H264BSFContext { uint8_t idr_sps_seen; uint8_t idr_pps_seen; int extradata_parsed; + int reset; } H264BSFContext; enum PsSource { @@ -261,7 +262,8 @@ static int h264_mp4toannexb_filter_ps(H264BSFContext *s, return 0; } -static int h264_mp4toannexb_init(AVBSFContext *ctx) +static int h264_mp4toannexb_init_extradata(AVBSFContext *ctx, + uint8_t **out, int *out_size) { int extra_size = ctx->par_in->extradata_size; @@ -276,10 +278,17 @@ static int h264_mp4toannexb_init(AVBSFContext *ctx) return h264_extradata_to_annexb(ctx, ctx->par_in->extradata, ctx->par_in->extradata_size, - &ctx->par_out->extradata, - &ctx->par_out->extradata_size); + out, out_size); } +static int h264_mp4toannexb_init(AVBSFContext *ctx) +{ + return h264_mp4toannexb_init_extradata(ctx, &ctx->par_out->extradata, + &ctx->par_out->extradata_size); +} + +static void h264_mp4toannexb_flush(AVBSFContext *ctx); + static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt) { H264BSFContext *s = ctx->priv_data; @@ -297,6 +306,17 @@ static int h264_mp4toannexb_filter(AVBSFContext *ctx, AVPacket *opkt) if (ret < 0) return ret; + if (s->reset) { + /* Flush cannot report allocation failures. Restore the initial state + * here, before applying any new extradata carried by this packet. */ + ret = h264_mp4toannexb_init_extradata(ctx, NULL, NULL); + if (ret < 0) { + h264_mp4toannexb_flush(ctx); + goto fail; + } + s->reset = 0; + } + extradata = av_packet_get_side_data(in, AV_PKT_DATA_NEW_EXTRADATA, &extradata_size); if (extradata && extradata[0] == 1) { @@ -459,9 +479,9 @@ static void h264_mp4toannexb_flush(AVBSFContext *ctx) { H264BSFContext *s = ctx->priv_data; - s->idr_sps_seen = 0; - s->idr_pps_seen = 0; - s->new_idr = s->extradata_parsed; + h264_mp4toannexb_close(ctx); + memset(s, 0, sizeof(*s)); + s->reset = 1; } static const enum AVCodecID codec_ids[] = { diff --git a/tests/api/Makefile b/tests/api/Makefile index e51dc6c5837..da7a8dfba03 100644 --- a/tests/api/Makefile +++ b/tests/api/Makefile @@ -1,3 +1,4 @@ +APITESTPROGS-$(CONFIG_H264_MP4TOANNEXB_BSF) += api-h264-mp4toannexb-flush APITESTPROGS-$(call ALLYES, LIBASTCENC_ENCODER LIBASTCENC_DECODER) += api-astc-profile APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264 diff --git a/tests/api/api-h264-mp4toannexb-flush-test.c b/tests/api/api-h264-mp4toannexb-flush-test.c new file mode 100644 index 00000000000..d6e5e8b2c5b --- /dev/null +++ b/tests/api/api-h264-mp4toannexb-flush-test.c @@ -0,0 +1,260 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <stdio.h> +#include <string.h> + +#include "libavcodec/bsf.h" +#include "libavcodec/defs.h" +#include "libavcodec/packet.h" +#include "libavutil/error.h" +#include "libavutil/intreadwrite.h" +#include "libavutil/log.h" +#include "libavutil/md5.h" +#include "libavutil/mem.h" + +/* A valid 16x16 black IDR picture generated with x264, baseline, QP 22. */ +static const uint8_t sps[] = { + 0x67, 0x42, 0xc0, 0x0a, 0xda, 0x7a, 0x10, 0x00, 0x00, 0x03, + 0x00, 0x10, 0x00, 0x00, 0x03, 0x03, 0x28, 0xf1, 0x22, 0x6a, +}; +static const uint8_t pps[] = { 0x68, 0xce, 0x04, 0xf2 }; +static const uint8_t idr[] = { + 0x65, 0x88, 0x84, 0x3a, 0x26, 0x28, 0x00, 0x09, 0x35, 0xe0, +}; +/* The same encoder settings with QP 24. */ +static const uint8_t pps_update[] = { 0x68, 0xce, 0x0b, 0xc8 }; + +static int make_avcc(uint8_t *dst, int have_sps, int have_pps, + int length_size, int update) +{ + int size = 6; + + dst[0] = 1; + memcpy(dst + 1, sps + 1, 3); + dst[4] = 0xfc | (length_size - 1); + dst[5] = 0xe0 | have_sps; + if (have_sps) { + AV_WB16(dst + size, sizeof(sps)); + size += 2; + memcpy(dst + size, sps, sizeof(sps)); + size += sizeof(sps); + } + dst[size++] = have_pps; + if (have_pps) { + AV_WB16(dst + size, sizeof(pps)); + size += 2; + memcpy(dst + size, update ? pps_update : pps, sizeof(pps)); + size += sizeof(pps); + } + return size; +} + +static int add_nal(AVPacket *pkt, const uint8_t *nal, int size, int length_size) +{ + int offset = pkt->size; + int prefix = length_size ? length_size : 4; + int ret = av_grow_packet(pkt, prefix + size); + + if (ret < 0) + return ret; + if (length_size == 2) + AV_WB16(pkt->data + offset, size); + else + AV_WB32(pkt->data + offset, length_size ? size : 1); + memcpy(pkt->data + offset + prefix, nal, size); + return 0; +} + +static int open_bsf(AVBSFContext **ctx, const uint8_t *extra, int size) +{ + int ret = av_bsf_alloc(av_bsf_get_by_name("h264_mp4toannexb"), ctx); + AVCodecParameters *par; + + if (ret < 0) + return ret; + par = (*ctx)->par_in; + par->codec_type = AVMEDIA_TYPE_VIDEO; + par->codec_id = AV_CODEC_ID_H264; + if (size) { + par->extradata = av_mallocz(size + AV_INPUT_BUFFER_PADDING_SIZE); + if (!par->extradata) + return AVERROR(ENOMEM); + memcpy(par->extradata, extra, size); + par->extradata_size = size; + } + return av_bsf_init(*ctx); +} + +static int filter(AVBSFContext *ctx, const AVPacket *input, AVPacket *output) +{ + AVPacket *copy = av_packet_clone(input); + int ret; + + if (!copy) + return AVERROR(ENOMEM); + ret = av_bsf_send_packet(ctx, copy); + av_packet_free(©); + if (ret < 0) + return ret; + return av_bsf_receive_packet(ctx, output); +} + +/* initial: 0..3 are avcC with a bit mask of SPS/PPS; 4 is empty; 5 Annex B. + * history: 1 SPS update, 2 PPS update, 3 both, 4 new avcC with 2-byte lengths. + */ +static int test(const char *name, int initial, int history, int new_extra) +{ + AVBSFContext *reused = NULL, *fresh = NULL; + AVPacket *hist = av_packet_alloc(), *input = av_packet_alloc(); + AVPacket *r = av_packet_alloc(), *f = av_packet_alloc(); + uint8_t extra[64], saved[64], updated_sps[sizeof(sps)], digest[16]; + int extra_size, saved_size, length_size, ret = AVERROR(ENOMEM); + + if (!hist || !input || !r || !f) + goto end; + if (initial < 4) { + extra_size = make_avcc(extra, initial & 1, !!(initial & 2), 4, 0); + } else if (initial == 4) { + extra_size = 0; + } else { + AV_WB32(extra, 1); + memcpy(extra + 4, sps, sizeof(sps)); + extra_size = 4 + sizeof(sps); + } + ret = open_bsf(&reused, extra, extra_size); + if (ret < 0) + goto end; + ret = open_bsf(&fresh, extra, extra_size); + if (ret < 0) + goto end; + saved_size = reused->par_out->extradata_size; + if (saved_size) + memcpy(saved, reused->par_out->extradata, saved_size); + + /* Flush before any packets, and flush twice, must also be harmless. */ + av_bsf_flush(reused); + av_bsf_flush(reused); + length_size = history == 4 ? 2 : 4; + if (history == 4) { + uint8_t *side; + extra_size = make_avcc(extra, 1, 1, length_size, 1); + side = av_packet_new_side_data(hist, AV_PKT_DATA_NEW_EXTRADATA, extra_size); + if (!side) { + ret = AVERROR(ENOMEM); + goto end; + } + memcpy(side, extra, extra_size); + } else { + if (history & 1) { + memcpy(updated_sps, sps, sizeof(sps)); + updated_sps[3] = 11; /* Change level_idc from 1.0 to 1.1. */ + ret = add_nal(hist, updated_sps, sizeof(updated_sps), length_size); + if (ret < 0) + goto end; + } + if (history & 2) { + ret = add_nal(hist, pps_update, sizeof(pps_update), length_size); + if (ret < 0) + goto end; + } + } + ret = add_nal(hist, idr, sizeof(idr), length_size); + if (ret < 0) + goto end; + ret = filter(reused, hist, r); + if (ret < 0) + goto end; + av_packet_unref(r); + ret = av_bsf_send_packet(reused, NULL); + if (ret < 0) + goto end; + av_bsf_flush(reused); + av_bsf_flush(reused); + + length_size = initial < 4 ? 4 : 0; + if (new_extra) { + uint8_t *side; + length_size = 2; + extra_size = make_avcc(extra, 1, 1, length_size, 1); + side = av_packet_new_side_data(input, AV_PKT_DATA_NEW_EXTRADATA, extra_size); + if (!side) { + ret = AVERROR(ENOMEM); + goto end; + } + memcpy(side, extra, extra_size); + } + ret = add_nal(input, idr, sizeof(idr), length_size); + if (ret < 0) + goto end; + ret = filter(reused, input, r); + if (ret < 0) + goto end; + ret = filter(fresh, input, f); + if (ret < 0) + goto end; + + printf("%s: size=%d equal=%d extradata_unchanged=%d md5=", name, r->size, + r->size == f->size && !memcmp(r->data, f->data, r->size), + saved_size == reused->par_out->extradata_size && + (!saved_size || !memcmp(saved, reused->par_out->extradata, saved_size))); + av_md5_sum(digest, r->data, r->size); + for (int i = 0; i < sizeof(digest); i++) + printf("%02x", digest[i]); + printf("\n"); + +end: + av_bsf_free(&reused); + av_bsf_free(&fresh); + av_packet_free(&hist); + av_packet_free(&input); + av_packet_free(&r); + av_packet_free(&f); + return ret; +} + +int main(void) +{ + static const struct { + const char *name; + int initial, history, new_extra; + } cases[] = { + { "in-band SPS", 3, 1, 0 }, + { "in-band PPS", 3, 2, 0 }, + { "in-band SPS and PPS", 3, 3, 0 }, + { "initial SPS only", 1, 3, 0 }, + { "initial PPS only", 2, 3, 0 }, + { "initial avcC without PS", 0, 3, 0 }, + { "new extradata history", 3, 4, 0 }, + { "initial empty", 4, 4, 0 }, + { "initial Annex B", 5, 4, 0 }, + { "new extradata on seek", 3, 4, 1 }, + { "empty then new extradata on seek", 4, 4, 1 }, + }; + + av_log_set_level(AV_LOG_ERROR); + for (int i = 0; i < sizeof(cases) / sizeof(*cases); i++) { + int ret = test(cases[i].name, cases[i].initial, cases[i].history, + cases[i].new_extra); + if (ret < 0) { + fprintf(stderr, "%s: %s\n", cases[i].name, av_err2str(ret)); + return 1; + } + } + return 0; +} diff --git a/tests/fate/api.mak b/tests/fate/api.mak index ca836a40e3d..c54dfc299c2 100644 --- a/tests/fate/api.mak +++ b/tests/fate/api.mak @@ -1,3 +1,7 @@ +FATE_API_LIBAVCODEC-$(CONFIG_H264_MP4TOANNEXB_BSF) += fate-api-h264-mp4toannexb-flush +fate-api-h264-mp4toannexb-flush: $(APITESTSDIR)/api-h264-mp4toannexb-flush-test$(EXESUF) +fate-api-h264-mp4toannexb-flush: CMD = run $(APITESTSDIR)/api-h264-mp4toannexb-flush-test$(EXESUF) + FATE_API_LIBAVCODEC-$(call ALLYES, LIBASTCENC_ENCODER LIBASTCENC_DECODER) += fate-api-astc-profile fate-api-astc-profile: $(APITESTSDIR)/api-astc-profile-test$(EXESUF) fate-api-astc-profile: CMD = run $(APITESTSDIR)/api-astc-profile-test$(EXESUF) diff --git a/tests/ref/fate/api-h264-mp4toannexb-flush b/tests/ref/fate/api-h264-mp4toannexb-flush new file mode 100644 index 00000000000..4d744f5521d --- /dev/null +++ b/tests/ref/fate/api-h264-mp4toannexb-flush @@ -0,0 +1,11 @@ +in-band SPS: size=45 equal=1 extradata_unchanged=1 md5=20a5a2fe909591b57978f0f9157e915f +in-band PPS: size=45 equal=1 extradata_unchanged=1 md5=20a5a2fe909591b57978f0f9157e915f +in-band SPS and PPS: size=45 equal=1 extradata_unchanged=1 md5=20a5a2fe909591b57978f0f9157e915f +initial SPS only: size=37 equal=1 extradata_unchanged=1 md5=597f52336ca1491f4cdaa5916d4555ea +initial PPS only: size=21 equal=1 extradata_unchanged=1 md5=c1cb2defebaac5d77484f06f41920485 +initial avcC without PS: size=14 equal=1 extradata_unchanged=1 md5=cad7cb6b989c312a7531ae2fcc78a333 +new extradata history: size=45 equal=1 extradata_unchanged=1 md5=20a5a2fe909591b57978f0f9157e915f +initial empty: size=14 equal=1 extradata_unchanged=1 md5=cad7cb6b989c312a7531ae2fcc78a333 +initial Annex B: size=14 equal=1 extradata_unchanged=1 md5=cad7cb6b989c312a7531ae2fcc78a333 +new extradata on seek: size=45 equal=1 extradata_unchanged=1 md5=4c174792172a38a02c9f556a7813a5c3 +empty then new extradata on seek: size=45 equal=1 extradata_unchanged=1 md5=4c174792172a38a02c9f556a7813a5c3 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
