PR #24820 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24820 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24820.patch
Starting a new ID3v2 tag currently retains the encoding context's previous length accumulator. Reusing a context after finishing an empty tag with 10 bytes of padding therefore writes a 20-byte tag whose header declares 30 bytes including the header. Reset `id3->len` in `ff_id3v2_start()` and clarify that the helper initializes the encoding context. Existing callers already provide zeroed contexts, so their output is unchanged. This addresses the internal initialization improvement requested in #24035 without changing the public muxer lifecycle contract. Add a sample-free FATE test for ID3v2.3 and ID3v2.4 using zeroed, reused, and explicitly nonzero length accumulators. Actual initialization and serialized-length results are compared against `tests/ref/fate/id3v2enc`. Validation on arm64, based on master 09f879b0742: - `make fate-id3v2enc` fails without the reset and passes with it. - The six existing ID3v2 FATE tests enabled in the minimal configuration pass with `/opt/fate-suite`. - Built the MP3, ADTS, AIFF, and OMA muxers. - `git diff --check` passes. >From c5948d3c60f87de69f26f913b926269f6cc7982d Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Tue, 29 Sep 2026 21:41:11 +0000 Subject: [PATCH] avformat/id3v2enc: initialize tag length in ff_id3v2_start Reset the length accumulator when starting a new tag, so the encoding context need not be zeroed by the caller and can be reused after finish. Existing callers already supply zeroed contexts, so their output is unchanged. Clarify that start initializes the encoding context. Add a sample-free FATE test for ID3v2.3 and ID3v2.4 with zeroed, reused, and explicitly nonzero length accumulators. Compare the declared tag length with the bytes written using a reference file. Addresses the internal initialization improvement requested in #24035; this does not change the public muxer lifecycle contract. Assisted-by: Fairy --- libavformat/Makefile | 1 + libavformat/id3v2.h | 2 +- libavformat/id3v2enc.c | 1 + libavformat/tests/.gitignore | 1 + libavformat/tests/id3v2enc.c | 56 ++++++++++++++++++++++++++++++++++++ tests/fate/libavformat.mak | 4 +++ tests/ref/fate/id3v2enc | 6 ++++ 7 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 libavformat/tests/id3v2enc.c create mode 100644 tests/ref/fate/id3v2enc diff --git a/libavformat/Makefile b/libavformat/Makefile index 437c3edb5a..bcd7b69ba6 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -799,6 +799,7 @@ TESTPROGS-$(CONFIG_HTTP_PROTOCOL) += http TESTPROGS-$(CONFIG_NETWORK) += noproxy TESTPROGS-$(CONFIG_SRTP) += srtp TESTPROGS-$(CONFIG_IMF_DEMUXER) += imf +TESTPROGS-$(CONFIG_MP3_MUXER) += id3v2enc TESTPROGS-$(CONFIG_OGG_MUXER) += ogg_pages TOOLS = aviocat \ diff --git a/libavformat/id3v2.h b/libavformat/id3v2.h index 77f4e6eb5b..c4a2a4f1a2 100644 --- a/libavformat/id3v2.h +++ b/libavformat/id3v2.h @@ -130,7 +130,7 @@ void ff_id3v2_read(AVFormatContext *s, const char *magic, ID3v2ExtraMeta **extra unsigned int max_search_size); /** - * Initialize an ID3v2 tag. + * Initialize the encoding context and write the header for a new ID3v2 tag. */ void ff_id3v2_start(ID3v2EncContext *id3, AVIOContext *pb, int id3v2_version, const char *magic); diff --git a/libavformat/id3v2enc.c b/libavformat/id3v2enc.c index 015a767202..9206af8837 100644 --- a/libavformat/id3v2enc.c +++ b/libavformat/id3v2enc.c @@ -350,6 +350,7 @@ void ff_id3v2_start(ID3v2EncContext *id3, AVIOContext *pb, int id3v2_version, const char *magic) { id3->version = id3v2_version; + id3->len = 0; avio_wb32(pb, MKBETAG(magic[0], magic[1], magic[2], id3v2_version)); avio_w8(pb, 0); diff --git a/libavformat/tests/.gitignore b/libavformat/tests/.gitignore index f7f9a868c7..0298cae0c9 100644 --- a/libavformat/tests/.gitignore +++ b/libavformat/tests/.gitignore @@ -1,4 +1,5 @@ /id3v2 +/id3v2enc /fifo_muxer /imf /mkdir diff --git a/libavformat/tests/id3v2enc.c b/libavformat/tests/id3v2enc.c new file mode 100644 index 0000000000..fcce8977f9 --- /dev/null +++ b/libavformat/tests/id3v2enc.c @@ -0,0 +1,56 @@ +/* + * 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 "libavutil/mem.h" +#include "libavformat/avio.h" +#include "libavformat/id3v2.h" + +int main(void) +{ + for (int version = 3; version <= 4; version++) { + ID3v2EncContext id3 = { 0 }; + + for (int tag = 0; tag < 3; tag++) { + AVIOContext *pb; + uint8_t *buf; + int initial_len, size; + + /* Test zeroed, reused, and explicitly nonzero contexts. */ + if (tag == 2) + id3.len = 123; + + if (avio_open_dyn_buf(&pb) < 0) + return 1; + + ff_id3v2_start(&id3, pb, version, ID3v2_DEFAULT_MAGIC); + initial_len = id3.len; + ff_id3v2_finish(&id3, pb, 10); + size = avio_close_dyn_buf(pb, &buf); + if (size < ID3v2_HEADER_SIZE) { + av_free(buf); + return 1; + } + printf("version=%d tag=%d initial_len=%d declared=%d actual=%d\n", + version, tag, initial_len, ff_id3v2_tag_len(buf), size); + av_free(buf); + } + } + return 0; +} diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak index f306e19245..9b6d059650 100644 --- a/tests/fate/libavformat.mak +++ b/tests/fate/libavformat.mak @@ -2,6 +2,10 @@ #fate-async: libavformat/tests/async$(EXESUF) #fate-async: CMD = run libavformat/tests/async +FATE_LIBAVFORMAT-$(CONFIG_MP3_MUXER) += fate-id3v2enc +fate-id3v2enc: libavformat/tests/id3v2enc$(EXESUF) +fate-id3v2enc: CMD = run libavformat/tests/id3v2enc$(EXESUF) + FATE_LIBAVFORMAT += fate-mkdir fate-mkdir: libavformat/tests/mkdir$(EXESUF) fate-mkdir: CMD = run libavformat/tests/mkdir$(EXESUF) diff --git a/tests/ref/fate/id3v2enc b/tests/ref/fate/id3v2enc new file mode 100644 index 0000000000..ec4f789226 --- /dev/null +++ b/tests/ref/fate/id3v2enc @@ -0,0 +1,6 @@ +version=3 tag=0 initial_len=0 declared=20 actual=20 +version=3 tag=1 initial_len=0 declared=20 actual=20 +version=3 tag=2 initial_len=0 declared=20 actual=20 +version=4 tag=0 initial_len=0 declared=20 actual=20 +version=4 tag=1 initial_len=0 declared=20 actual=20 +version=4 tag=2 initial_len=0 declared=20 actual=20 -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
