PR #25625 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25625 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25625.patch
With PARSER_FLAG_COMPLETE_FRAMES, short VC-1 frame headers remain buffered until the next call, and retained start-code scanning state can misinterpret bytes from the next frame. Incorrect picture types then affect timestamp inference and packet keyframe flags. Apply Fredrik Mellbin’s attached patch from #25565, preserving its authorship, and add a sample-free API/FATE regression test. Reset frame-local scanning state at each complete-frame call and extract pending short frame headers before returning, while preserving sequence and entry-point configuration. The regression is verified at a0d7f9ec9a10 against its immediate parent. On master 8e362b7b51e7, the issue sample has 66 mismatched timestamp pairs among 77 comparable frames; the fix reduces this to zero and restores the final packet’s keyframe flag. All 78 decoded frame hashes remain identical. Validation: `make -j8 fate-api-vc1-parser fate-vc1 SAMPLES=/opt/fate-suite` passes the new API test and all nine existing VC-1 tests on Linux AArch64. The new test covers short headers and false start-code detection across calls, with expected output in `tests/ref/fate/api-vc1-parser`. Its output differs from the reference on unpatched master and with either fix block applied alone. Fixes #25565. From fa24c67b573ff47fa2c77f4b0200e54975e63a14 Mon Sep 17 00:00:00 2001 From: Fredrik Mellbin <[email protected]> Date: Wed, 28 Oct 2026 15:58:05 +0100 Subject: [PATCH 1/2] avcodec/vc1_parser: reset the parse state for every complete frame With PARSER_FLAG_COMPLETE_FRAMES, which libavformat sets for Matroska, each call holds one whole frame. The parser still kept its unescaping state from the call before and skipped the first four bytes of every frame as the start code it had already seen. That breaks in two ways: - A frame header shorter than UNESCAPED_THRESHOLD, as in a five byte skipped P-frame, is never extracted in its own call. Its bytes stay in unesc_buffer and are read as the start of the next frame's header, so the frame after a skipped one is reported with the skipped frame's type, and the skipped frame with the type of the one before it. - An unescaped header ending in zero bytes leaves search_state at ONE_ZERO or TWO_ZEROS, and a following frame whose header starts with 0x01 completes a start code that isn't there. prev_start_code takes the byte after it, the complete frames path only extracts a header after a frame start code, and none is read for the rest of the stream. libavformat derives the PTS of B-frames and the key flag from these picture types, so packets get wrong timestamps and flags, and -fflags +genpts produces timestamps out of presentation order. In two WVC1 Matroska files written by mkvmerge, the parser reported a different picture type than the decoder gives the frame for 50 of 1440 and 757 of 1450 packets, and +genpts left 44 and 696 frames out of order. After this change none are, and remuxing either with +genpts gives every frame its presentation timestamp, with the decoded frames unchanged. Start every complete frame from a fresh state, scanning its own start code, and extract a frame header still pending when the frame ends. Co-Authored-By: Claude Opus 5.5 <[email protected]> Assisted-by: Fairy --- libavcodec/vc1_parser.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/libavcodec/vc1_parser.c b/libavcodec/vc1_parser.c index 224ff3ee587..40c9d9c8d99 100644 --- a/libavcodec/vc1_parser.c +++ b/libavcodec/vc1_parser.c @@ -169,6 +169,18 @@ static int vc1_parse(AVCodecParserContext *s, int next = END_NOT_FOUND; int i = vpc->bytes_to_skip; + if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) { + /* Every call holds one whole frame, so start it from scratch and scan + * its own start code. Nothing may carry over from the previous frame: + * neither a header too short to reach the threshold, nor zeros at its + * end that the first bytes of this one would complete into a start + * code. */ + unesc_index = 0; + search_state = NO_MATCH; + vpc->prev_start_code = 0; + i = 0; + } + if (!vpc->extradata_parsed) { vpc->extradata_parsed = 1; vc1_parse_extradata(s, avctx); @@ -259,6 +271,16 @@ static int vc1_parse(AVCodecParserContext *s, } } + if ((s->flags & PARSER_FLAG_COMPLETE_FRAMES) && unesc_index && + vpc->prev_start_code == (VC1_CODE_FRAME & 0xFF)) { + // A complete frame too short to reach the threshold, such as a + // skipped frame, ends here. Read its header now, as at EOF, instead + // of letting its bytes prefix the header of the next frame. + memset(unesc_buffer + unesc_index, 0, UNESCAPED_THRESHOLD - unesc_index); + vc1_extract_header(s, avctx, unesc_buffer, unesc_index); + unesc_index = 0; + } + vpc->pc.frame_start_found = pic_found; vpc->unesc_index = unesc_index; vpc->search_state = search_state; -- 2.54.0 From eb0cd0b1904cdf271bfefaee1fe6388fdd4eb84d Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Fri, 30 Oct 2026 18:59:13 +0000 Subject: [PATCH 2/2] tests/api: cover complete-frame VC-1 parser state Exercise skipped P-frames and short B-frame headers, and a header ending in two zeros followed by a header beginning with 0x01. Check both the immediate frame type and the next frame so that stale buffered bytes and false start-code detection are independently covered. Use synthetic headers and print parser results for comparison against a FATE reference, without requiring a new external sample. The test differs from the reference on unpatched master and with either half of the complete-frame fix applied alone. It passes with the full fix, as do all nine existing fate-vc1 tests. Assisted-by: Fairy --- tests/api/Makefile | 1 + tests/api/api-vc1-parser-test.c | 89 +++++++++++++++++++++++++++++++++ tests/fate/api.mak | 4 ++ tests/ref/fate/api-vc1-parser | 8 +++ 4 files changed, 102 insertions(+) create mode 100644 tests/api/api-vc1-parser-test.c create mode 100644 tests/ref/fate/api-vc1-parser diff --git a/tests/api/Makefile b/tests/api/Makefile index c411e82e5f1..1c958924b5d 100644 --- a/tests/api/Makefile +++ b/tests/api/Makefile @@ -12,6 +12,7 @@ APITESTPROGS-$(HAVE_THREADS) += api-threadmessage APITESTPROGS-$(CONFIG_AVCODEC) += api-enc-info APITESTPROGS-$(CONFIG_PCM_S16LE_DECODER) += api-skip-samples APITESTPROGS-$(call ALLYES, H261_ENCODER H261_PARSER) += api-enc-parser +APITESTPROGS-$(CONFIG_VC1_PARSER) += api-vc1-parser APITESTPROGS += $(APITESTPROGS-yes) APITESTOBJS := $(APITESTOBJS:%=$(APITESTSDIR)%) $(APITESTPROGS:%=$(APITESTSDIR)/%-test.o) diff --git a/tests/api/api-vc1-parser-test.c b/tests/api/api-vc1-parser-test.c new file mode 100644 index 00000000000..1d59b772dda --- /dev/null +++ b/tests/api/api-vc1-parser-test.c @@ -0,0 +1,89 @@ +/* + * 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/avcodec.h" +#include "libavutil/avutil.h" + +/* Synthetic advanced-profile headers for a progressive 16x16 sequence, + * with pulldown enabled and no optional display or HRD information. + * Macroblock data is unnecessary: the parser only reads frame headers. */ +static const uint8_t sequence[] = { + 0, 0, 1, 0x0f, 0xc2, 0, 0, 0x70, 0x07, 0x88, + 0, 0, 1, 0x0e, 0, 0, +}; + +static void parse(AVCodecParserContext *parser, AVCodecContext *ctx, + const char *name, int init, uint8_t header, + int payload_size, int trailing_zeros) +{ + uint8_t buf[128 + AV_INPUT_BUFFER_PADDING_SIZE] = { 0 }; + uint8_t *out; + int size = 0, out_size, ret; + + if (init) { + memcpy(buf, sequence, sizeof(sequence)); + size += sizeof(sequence); + } + buf[size++] = 0; + buf[size++] = 0; + buf[size++] = 1; + buf[size++] = 0x0d; + memset(buf + size, 0xff, payload_size); + buf[size] = header; + if (payload_size > 1) + buf[size + 1] = header == 0x80 ? 0x80 : 0; + size += payload_size; + memset(buf + size - trailing_zeros, 0, trailing_zeros); + + ret = av_parser_parse2(parser, ctx, &out, &out_size, buf, size, + AV_NOPTS_VALUE, AV_NOPTS_VALUE, 0); + printf("%s: consumed=%d size=%d type=%c repeat=%d\n", + name, ret, out_size, av_get_picture_type_char(parser->pict_type), + parser->repeat_pict); +} + +int main(void) +{ + AVCodecParserContext *parser = av_parser_init(AV_CODEC_ID_VC1); + AVCodecContext *ctx = avcodec_alloc_context3(NULL); + + if (!parser || !ctx) { + av_parser_close(parser); + avcodec_free_context(&ctx); + return 1; + } + parser->flags |= PARSER_FLAG_COMPLETE_FRAMES; + ctx->codec_id = AV_CODEC_ID_VC1; + ctx->codec_type = AVMEDIA_TYPE_VIDEO; + + parse(parser, ctx, "initial I", 1, 0xc1, 37, 0); + parse(parser, ctx, "skipped P", 0, 0xf0, 1, 0); + parse(parser, ctx, "B after skipped P", 0, 0x80, 37, 0); + parse(parser, ctx, "I ending in zeros", 0, 0xc1, 37, 2); + parse(parser, ctx, "P starting in 01", 0, 0x01, 37, 0); + parse(parser, ctx, "I after P", 0, 0xc1, 37, 0); + parse(parser, ctx, "short B", 0, 0x80, 2, 0); + parse(parser, ctx, "final skipped P", 0, 0xf0, 1, 0); + + av_parser_close(parser); + avcodec_free_context(&ctx); + return 0; +} diff --git a/tests/fate/api.mak b/tests/fate/api.mak index 1659c7979c0..c41f06c400b 100644 --- a/tests/fate/api.mak +++ b/tests/fate/api.mak @@ -1,3 +1,7 @@ +FATE_API_LIBAVCODEC-$(CONFIG_VC1_PARSER) += fate-api-vc1-parser +fate-api-vc1-parser: $(APITESTSDIR)/api-vc1-parser-test$(EXESUF) +fate-api-vc1-parser: CMD = run $(APITESTSDIR)/api-vc1-parser-test$(EXESUF) + FATE_API_LIBAVCODEC-$(call ALLYES, AC3_ENCODER AC3_DECODER EAC3_DECODER AC3_FIXED_DECODER) += fate-api-ac3-flush fate-api-ac3-flush: $(APITESTSDIR)/api-ac3-flush-test$(EXESUF) fate-api-ac3-flush: CMD = run $(APITESTSDIR)/api-ac3-flush-test$(EXESUF) diff --git a/tests/ref/fate/api-vc1-parser b/tests/ref/fate/api-vc1-parser new file mode 100644 index 00000000000..b46dbbb9677 --- /dev/null +++ b/tests/ref/fate/api-vc1-parser @@ -0,0 +1,8 @@ +initial I: consumed=57 size=57 type=I repeat=1 +skipped P: consumed=5 size=5 type=P repeat=1 +B after skipped P: consumed=41 size=41 type=B repeat=1 +I ending in zeros: consumed=41 size=41 type=I repeat=1 +P starting in 01: consumed=41 size=41 type=P repeat=1 +I after P: consumed=41 size=41 type=I repeat=1 +short B: consumed=6 size=6 type=B repeat=1 +final skipped P: consumed=5 size=5 type=P repeat=1 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
