This is an automated email from the git hooks/post-receive script.
Git pushed a commit to branch master
in repository ffmpeg.
The following commit(s) were added to refs/heads/master by this push:
new ca99230f71 avformat/mp3enc: buffer ID3v2 for non-seekable outputs
ca99230f71 is described below
commit ca99230f7176c589b8390b34f2d1c130b9e1881d
Author: iSold Leo <[email protected]>
AuthorDate: Mon Aug 3 22:48:32 2026 +0800
Commit: michaelni <[email protected]>
CommitDate: Mon Aug 24 20:09:35 2026 +0000
avformat/mp3enc: buffer ID3v2 for non-seekable outputs
ID3v2 writes the tag size by seeking back to its header. This fails after
output has been flushed to a non-seekable AVIO context, leaving a zero-sized
tag. Attached pictures expose this reliably; sufficiently large metadata is
affected too.
Build the tag in a dynamic buffer for non-seekable MP3 outputs and copy it
after finalization. Keep the direct path for seekable outputs.
Fixes: #23917
Signed-off-by: iSold Leo <[email protected]>
---
libavformat/aiffenc.c | 4 +--
libavformat/id3v2.h | 6 +++--
libavformat/id3v2enc.c | 32 +++++++++++++-----------
libavformat/mp3enc.c | 64 +++++++++++++++++++++++++++++++++++++++++-------
tests/fate/cover-art.mak | 6 +++++
tests/fate/id3v2.mak | 5 ++++
6 files changed, 90 insertions(+), 27 deletions(-)
diff --git a/libavformat/aiffenc.c b/libavformat/aiffenc.c
index 3368e28404..e27a2ca5e6 100644
--- a/libavformat/aiffenc.c
+++ b/libavformat/aiffenc.c
@@ -59,9 +59,9 @@ static int put_id3v2_tags(AVFormatContext *s,
AIFFOutputContext *aiff)
pos = avio_tell(pb);
ff_id3v2_start(&id3v2, pb, aiff->id3v2_version, ID3v2_DEFAULT_MAGIC);
- ff_id3v2_write_metadata(s, &id3v2);
+ ff_id3v2_write_metadata(s, pb, &id3v2);
while (list_entry) {
- if ((ret = ff_id3v2_write_apic(s, &id3v2, &list_entry->pkt)) < 0)
+ if ((ret = ff_id3v2_write_apic(s, pb, &id3v2, &list_entry->pkt)) < 0)
return ret;
list_entry = list_entry->next;
}
diff --git a/libavformat/id3v2.h b/libavformat/id3v2.h
index c55cf52128..77f4e6eb5b 100644
--- a/libavformat/id3v2.h
+++ b/libavformat/id3v2.h
@@ -138,12 +138,14 @@ void ff_id3v2_start(ID3v2EncContext *id3, AVIOContext
*pb, int id3v2_version,
/**
* Convert and write all global metadata from s into an ID3v2 tag.
*/
-int ff_id3v2_write_metadata(AVFormatContext *s, ID3v2EncContext *id3);
+int ff_id3v2_write_metadata(AVFormatContext *s, AVIOContext *pb,
+ ID3v2EncContext *id3);
/**
* Write an attached picture from pkt into an ID3v2 tag.
*/
-int ff_id3v2_write_apic(AVFormatContext *s, ID3v2EncContext *id3, AVPacket
*pkt);
+int ff_id3v2_write_apic(AVFormatContext *s, AVIOContext *pb,
+ ID3v2EncContext *id3, AVPacket *pkt);
/**
* Finalize an opened ID3v2 tag.
diff --git a/libavformat/id3v2enc.c b/libavformat/id3v2enc.c
index 12b0400541..015a767202 100644
--- a/libavformat/id3v2enc.c
+++ b/libavformat/id3v2enc.c
@@ -408,7 +408,8 @@ static int write_metadata(AVIOContext *pb, AVDictionary
**metadata,
return 0;
}
-static int write_ctoc(AVFormatContext *s, ID3v2EncContext *id3, int enc)
+static int write_ctoc(AVFormatContext *s, AVIOContext *pb,
+ ID3v2EncContext *id3, int enc)
{
AVIOContext *dyn_bc;
char name[123];
@@ -428,11 +429,12 @@ static int write_ctoc(AVFormatContext *s, ID3v2EncContext
*id3, int enc)
avio_put_str(dyn_bc, name);
}
- return id3v2_put_frame(id3, s->pb, dyn_bc,
+ return id3v2_put_frame(id3, pb, dyn_bc,
MKBETAG('C', 'T', 'O', 'C'), 0);
}
-static int write_chapter(AVFormatContext *s, ID3v2EncContext *id3, int id, int
enc)
+static int write_chapter(AVFormatContext *s, AVIOContext *pb,
+ ID3v2EncContext *id3, int id, int enc)
{
const AVRational time_base = {1, 1000};
AVChapter *ch = s->chapters[id];
@@ -460,10 +462,10 @@ static int write_chapter(AVFormatContext *s,
ID3v2EncContext *id3, int id, int e
len = avio_get_dyn_buf(dyn_bc, &dyn_buf);
id3->len += 16 + ID3v2_HEADER_SIZE;
- avio_wb32(s->pb, MKBETAG('C', 'H', 'A', 'P'));
- avio_wb32(s->pb, len);
- avio_wb16(s->pb, 0);
- avio_write(s->pb, dyn_buf, len);
+ avio_wb32(pb, MKBETAG('C', 'H', 'A', 'P'));
+ avio_wb32(pb, len);
+ avio_wb16(pb, 0);
+ avio_write(pb, dyn_buf, len);
fail:
ffio_free_dyn_buf(&dyn_bc);
@@ -471,28 +473,30 @@ fail:
return ret;
}
-int ff_id3v2_write_metadata(AVFormatContext *s, ID3v2EncContext *id3)
+int ff_id3v2_write_metadata(AVFormatContext *s, AVIOContext *pb,
+ ID3v2EncContext *id3)
{
int enc = id3->version == 3 ? ID3v2_ENCODING_UTF16BOM :
ID3v2_ENCODING_UTF8;
int i, ret;
ff_standardize_creation_time(s);
- if ((ret = write_metadata(s->pb, &s->metadata, id3, enc)) < 0)
+ if ((ret = write_metadata(pb, &s->metadata, id3, enc)) < 0)
return ret;
- if ((ret = write_ctoc(s, id3, enc)) < 0)
+ if ((ret = write_ctoc(s, pb, id3, enc)) < 0)
return ret;
for (i = 0; i < s->nb_chapters; i++) {
- if ((ret = write_chapter(s, id3, i, enc)) < 0)
+ if ((ret = write_chapter(s, pb, id3, i, enc)) < 0)
return ret;
}
return 0;
}
-int ff_id3v2_write_apic(AVFormatContext *s, ID3v2EncContext *id3, AVPacket
*pkt)
+int ff_id3v2_write_apic(AVFormatContext *s, AVIOContext *pb,
+ ID3v2EncContext *id3, AVPacket *pkt)
{
AVStream *st = s->streams[pkt->stream_index];
AVDictionaryEntry *e;
@@ -545,7 +549,7 @@ int ff_id3v2_write_apic(AVFormatContext *s, ID3v2EncContext
*id3, AVPacket *pkt)
id3v2_encode_string(dyn_buf, desc, enc);
avio_write(dyn_buf, pkt->data, pkt->size);
- return id3v2_put_frame(id3, s->pb, dyn_buf,
+ return id3v2_put_frame(id3, pb, dyn_buf,
MKBETAG('A', 'P', 'I', 'C'), 0);
}
@@ -580,7 +584,7 @@ int ff_id3v2_write_simple(struct AVFormatContext *s, int
id3v2_version,
int ret;
ff_id3v2_start(&id3, s->pb, id3v2_version, magic);
- if ((ret = ff_id3v2_write_metadata(s, &id3)) < 0)
+ if ((ret = ff_id3v2_write_metadata(s, s->pb, &id3)) < 0)
return ret;
ff_id3v2_finish(&id3, s->pb, s->metadata_header_padding);
diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
index 14eac3dab6..6d42feb1ff 100644
--- a/libavformat/mp3enc.c
+++ b/libavformat/mp3enc.c
@@ -100,6 +100,7 @@ static int id3v1_create_tag(AVFormatContext *s, uint8_t
*buf)
typedef struct MP3Context {
const AVClass *class;
ID3v2EncContext id3;
+ AVIOContext *id3_pb;
int id3v2_version;
int write_id3v1;
int write_xing;
@@ -381,14 +382,41 @@ static int mp3_write_audio_packet(AVFormatContext *s,
AVPacket *pkt)
return ff_raw_write_packet(s, pkt);
}
+static int mp3_finish_id3v2(AVFormatContext *s)
+{
+ MP3Context *mp3 = s->priv_data;
+ AVIOContext *pb = mp3->id3_pb ? mp3->id3_pb : s->pb;
+ uint8_t *buf = NULL;
+ int ret, size;
+
+ ff_id3v2_finish(&mp3->id3, pb, s->metadata_header_padding);
+ ret = pb->error;
+
+ if (!mp3->id3_pb)
+ return ret;
+
+ size = avio_get_dyn_buf(mp3->id3_pb, &buf);
+ ret = mp3->id3_pb->error;
+ if (ret >= 0) {
+ avio_write(s->pb, buf, size);
+ ret = s->pb->error;
+ }
+ ffio_free_dyn_buf(&mp3->id3_pb);
+
+ return ret;
+}
+
static int mp3_queue_flush(AVFormatContext *s)
{
MP3Context *mp3 = s->priv_data;
AVPacket *const pkt = ffformatcontext(s)->pkt;
int ret = 0, write = 1;
- ff_id3v2_finish(&mp3->id3, s->pb, s->metadata_header_padding);
- mp3_write_xing(s);
+ ret = mp3_finish_id3v2(s);
+ if (ret < 0)
+ write = 0;
+ else
+ mp3_write_xing(s);
while (mp3->queue.head) {
ff_packet_list_get(&mp3->queue, pkt);
@@ -478,11 +506,13 @@ static int mp3_write_trailer(struct AVFormatContext *s)
{
uint8_t buf[ID3v1_TAG_SIZE];
MP3Context *mp3 = s->priv_data;
+ int ret;
if (mp3->pics_to_write) {
av_log(s, AV_LOG_WARNING, "No packets were sent for some of the "
"attached pictures.\n");
- mp3_queue_flush(s);
+ if ((ret = mp3_queue_flush(s)) < 0)
+ return ret;
}
/* write the id3v1 tag */
@@ -540,12 +570,14 @@ static int mp3_write_packet(AVFormatContext *s, AVPacket
*pkt)
if (ret < 0) {
av_log(s, AV_LOG_WARNING, "Not enough memory to buffer audio.
Skipping picture streams\n");
mp3->pics_to_write = 0;
- mp3_queue_flush(s);
+ if ((ret = mp3_queue_flush(s)) < 0)
+ return ret;
return mp3_write_audio_packet(s, pkt);
}
} else
return mp3_write_audio_packet(s, pkt);
} else {
+ AVIOContext *pb = mp3->id3_pb ? mp3->id3_pb : s->pb;
int ret;
/* warn only once for each stream */
@@ -556,8 +588,11 @@ static int mp3_write_packet(AVFormatContext *s, AVPacket
*pkt)
if (!mp3->pics_to_write || s->streams[pkt->stream_index]->nb_frames >=
1)
return 0;
- if ((ret = ff_id3v2_write_apic(s, &mp3->id3, pkt)) < 0)
+ ret = ff_id3v2_write_apic(s, pb, &mp3->id3, pkt);
+ if (ret < 0)
return ret;
+ if (pb->error < 0)
+ return pb->error;
mp3->pics_to_write--;
/* flush the buffered audio packets */
@@ -621,18 +656,28 @@ static int mp3_init(struct AVFormatContext *s)
static int mp3_write_header(struct AVFormatContext *s)
{
MP3Context *mp3 = s->priv_data;
+ AVIOContext *pb = s->pb;
int ret;
if (mp3->id3v2_version) {
- ff_id3v2_start(&mp3->id3, s->pb, mp3->id3v2_version,
ID3v2_DEFAULT_MAGIC);
- ret = ff_id3v2_write_metadata(s, &mp3->id3);
+ if (!(s->pb->seekable & AVIO_SEEKABLE_NORMAL)) {
+ ret = avio_open_dyn_buf(&mp3->id3_pb);
+ if (ret < 0)
+ return ret;
+ pb = mp3->id3_pb;
+ }
+
+ ff_id3v2_start(&mp3->id3, pb, mp3->id3v2_version, ID3v2_DEFAULT_MAGIC);
+ ret = ff_id3v2_write_metadata(s, pb, &mp3->id3);
if (ret < 0)
return ret;
+ if (pb->error < 0)
+ return pb->error;
}
if (!mp3->pics_to_write) {
- if (mp3->id3v2_version)
- ff_id3v2_finish(&mp3->id3, s->pb, s->metadata_header_padding);
+ if (mp3->id3v2_version && (ret = mp3_finish_id3v2(s)) < 0)
+ return ret;
mp3_write_xing(s);
}
@@ -644,6 +689,7 @@ static void mp3_deinit(struct AVFormatContext *s)
MP3Context *mp3 = s->priv_data;
ff_packet_list_free(&mp3->queue);
+ ffio_free_dyn_buf(&mp3->id3_pb);
av_freep(&mp3->xing_frame);
}
diff --git a/tests/fate/cover-art.mak b/tests/fate/cover-art.mak
index b56d254bdd..febdec6f04 100644
--- a/tests/fate/cover-art.mak
+++ b/tests/fate/cover-art.mak
@@ -46,6 +46,12 @@ FATE_COVER_ART_REMUX-$(call ALLYES, MP3_DEMUXER
MJPEG_DECODER \
+= fate-cover-art-mp3-id3v2-remux
fate-cover-art-mp3-id3v2-remux: CMD = transcode mp3
$(TARGET_SAMPLES)/exif/embedded_small.mp3 mp3 "-map 0 -map 0:v -map 0:v -c:a
copy -filter:v:0 scale -filter:v:2 scale -c:v:0 bmp -c:v:1 copy -c:v:2 png
-metadata:s:v:0 comment=Band/Orchestra" "-map 0 -c copy -t 0.1" "-show_entries
stream_tags:stream_disposition=attached_pic:stream=index,codec_name"
+FATE_COVER_ART_REMUX-$(call ALLYES, MP3_DEMUXER MP3_MUXER MD5_PROTOCOL) \
+ += fate-cover-art-mp3-id3v2-nonseekable
+fate-cover-art-mp3-id3v2-nonseekable: CMD = md5pipe -i
$(TARGET_SAMPLES)/exif/embedded_small.mp3 -map 0 -c copy -fflags +bitexact
-write_xing 0 -f mp3
+fate-cover-art-mp3-id3v2-nonseekable: CMP = oneline
+fate-cover-art-mp3-id3v2-nonseekable: REF = 7d7fc4c0a5fe89a418bab6c74b6cda08
+
# Also covers muxing and demuxing of nonstandard channel layouts into FLAC
# as well as the unorthodox multi_dim_quant option of the FLAC encoder.
FATE_COVER_ART_REMUX-$(call ALLYES, MOV_DEMUXER OGG_DEMUXER \
diff --git a/tests/fate/id3v2.mak b/tests/fate/id3v2.mak
index 46e38d888d..641d930b9d 100644
--- a/tests/fate/id3v2.mak
+++ b/tests/fate/id3v2.mak
@@ -18,6 +18,11 @@ fate-id3v2-keep-metadata-invalid-stream-spec: REF = Trailing
garbage at the end
FATE_ID3V2_FFMPEG_FFPROBE-$(call REMUX, MP3) += fate-id3v2-priv-remux
fate-id3v2-priv-remux: CMD = transcode mp3
$(TARGET_SAMPLES)/id3v2/id3v2_priv.mp3 mp3 "-c copy" "-c copy -t 0.1"
"-show_entries format_tags"
+FATE_ID3V2_FFMPEG-$(call ALLYES, MP3_DEMUXER MP3_MUXER MD5_PROTOCOL) +=
fate-id3v2-padding-nonseekable
+fate-id3v2-padding-nonseekable: CMD = md5pipe -i
$(TARGET_SAMPLES)/exif/embedded_small.mp3 -map 0:a -map_metadata -1 -metadata
title=padding-test -c copy -fflags +bitexact -write_xing 0
-metadata_header_padding 40000 -f mp3
+fate-id3v2-padding-nonseekable: CMP = oneline
+fate-id3v2-padding-nonseekable: REF = 6a53649febb6345d3752445dcd3f3159
+
ID3V2_TESTBIN = libavformat/tests/id3v2$(EXESUF)
FATE_ID3V2_RAW-$(call REMUX, MP3) += fate-id3v2-comm
--
To stop receiving notification emails like this one, please contact
[email protected].
_______________________________________________
ffmpeg-cvslog mailing list -- [email protected]
To unsubscribe send an email to [email protected]
