@@ -96,6 +96,10 @@ static int id3v1_create_tag(AVFormatContext *s, uint8_t *buf)
// size of the XING/LAME data, starting from the Xing tag
#define XING_SIZE 156
+typedef struct MP3StreamData {
+ AVPacket *apic;
+} MP3StreamData;
+
typedef struct MP3Context {
const AVClass *class;
ID3v2EncContext id3;
@@ -129,8 +133,8 @@ typedef struct MP3Context {
/* index of the audio stream */
int audio_stream_idx;
- /* number of attached pictures we still need to write */
- int pics_to_write;
+ /* number of attached pictures for which we did not yet get a packet */
+ int pics_to_buffer;
/* audio packets are queued here until we get all the attached pictures */
PacketList queue;
@@ -385,6 +389,20 @@ static int mp3_queue_flush(AVFormatContext *s)
AVPacket *const pkt = ffformatcontext(s)->pkt;
int ret = 0, write = 1;
+ // write all buffered attached pictures
+ for (int i = 0; i < s->nb_streams; i++) {
+ MP3StreamData *stp = s->streams[i]->priv_data;
+
+ if (!stp || !stp->apic)
+ continue;
+
+ ret = ff_id3v2_write_apic(s, &mp3->id3, stp->apic);
+ if (ret < 0)
+ return ret;
+
+ av_packet_free(&stp->apic);
+ }
+
ff_id3v2_finish(&mp3->id3, s->pb, s->metadata_header_padding);
mp3_write_xing(s);
@@ -473,7 +491,7 @@ static int mp3_write_trailer(struct AVFormatContext *s)
uint8_t buf[ID3v1_TAG_SIZE];
MP3Context *mp3 = s->priv_data;
- if (mp3->pics_to_write) {
+ if (mp3->pics_to_buffer) {
av_log(s, AV_LOG_WARNING, "No packets were sent for some of the "
"attached pictures.\n");
mp3_queue_flush(s);
@@ -523,35 +541,40 @@ static int mp3_write_packet(AVFormatContext *s, AVPacket *pkt)
MP3Context *mp3 = s->priv_data;
if (pkt->stream_index == mp3->audio_stream_idx) {
- if (mp3->pics_to_write) {
+ if (mp3->pics_to_buffer) {
/* buffer audio packets until we get all the pictures */
int ret = avpriv_packet_list_put(&mp3->queue, pkt, NULL, 0);
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->pics_to_buffer = 0;
mp3_queue_flush(s);
return mp3_write_audio_packet(s, pkt);
}
} else
return mp3_write_audio_packet(s, pkt);
} else {
+ AVStream *st = s->streams[pkt->stream_index];
+ MP3StreamData *stp = st->priv_data;
int ret;
/* warn only once for each stream */
- if (s->streams[pkt->stream_index]->nb_frames == 1) {
+ if (st->nb_frames == 1) {
av_log(s, AV_LOG_WARNING, "Got more than one picture in stream %d,"
" ignoring.\n", pkt->stream_index);
}
- if (!mp3->pics_to_write || s->streams[pkt->stream_index]->nb_frames >= 1)
+ if (!mp3->pics_to_buffer || st->nb_frames >= 1)
return 0;
- if ((ret = ff_id3v2_write_apic(s, &mp3->id3, pkt)) < 0)
- return ret;
- mp3->pics_to_write--;
+ av_assert0(!stp->apic);
+ stp->apic = av_packet_clone(pkt);
+ if (!stp->apic)
+ return AVERROR(ENOMEM);
+
+ mp3->pics_to_buffer--;
/* flush the buffered audio packets */
- if (!mp3->pics_to_write &&
+ if (!mp3->pics_to_buffer &&
(ret = mp3_queue_flush(s)) < 0)
return ret;
}
@@ -588,7 +611,11 @@ static int mp3_init(struct AVFormatContext *s)
return AVERROR(EINVAL);
}
mp3->audio_stream_idx = i;
- } else if (st->codecpar->codec_type != AVMEDIA_TYPE_VIDEO) {
+ } else if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+ st->priv_data = av_mallocz(sizeof(MP3StreamData));
+ if (!st->priv_data)
+ return AVERROR(ENOMEM);
+ } else {
av_log(s, AV_LOG_ERROR, "Only audio streams and pictures are allowed in MP3.\n");
return AVERROR(EINVAL);
}
@@ -597,9 +624,9 @@ static int mp3_init(struct AVFormatContext *s)
av_log(s, AV_LOG_ERROR, "No audio stream present.\n");
return AVERROR(EINVAL);
}
- mp3->pics_to_write = s->nb_streams - 1;
+ mp3->pics_to_buffer = s->nb_streams - 1;
- if (mp3->pics_to_write && !mp3->id3v2_version) {
+ if (mp3->pics_to_buffer && !mp3->id3v2_version) {
av_log(s, AV_LOG_ERROR, "Attached pictures were requested, but the "
"ID3v2 header is disabled.\n");
return AVERROR(EINVAL);
@@ -620,7 +647,7 @@ static int mp3_write_header(struct AVFormatContext *s)
return ret;
}
- if (!mp3->pics_to_write) {
+ if (!mp3->pics_to_buffer) {
if (mp3->id3v2_version)
ff_id3v2_finish(&mp3->id3, s->pb, s->metadata_header_padding);
mp3_write_xing(s);
@@ -633,6 +660,13 @@ static void mp3_deinit(struct AVFormatContext *s)
{
MP3Context *mp3 = s->priv_data;
+ for (int i = 0; i < s->nb_streams; i++) {
+ MP3StreamData *stp = s->streams[i]->priv_data;
+
+ if (stp)
+ av_packet_free(&stp->apic);
+ }
+
avpriv_packet_list_free(&mp3->queue);
av_freep(&mp3->xing_frame);
}
@@ -1,4 +1,4 @@
-c1b55a9a92226cd72d3f53ccd830d127 *tests/data/fate/cover-art-mp3-id3v2-remux.mp3
+94946f0efd5f9bb0061ac1fbff7d731f *tests/data/fate/cover-art-mp3-id3v2-remux.mp3
399346 tests/data/fate/cover-art-mp3-id3v2-remux.mp3
#tb 0: 1/14112000
#media_type 0: audio
@@ -7,22 +7,22 @@ c1b55a9a92226cd72d3f53ccd830d127 *tests/data/fate/cover-art-mp3-id3v2-remux.mp3
#channel_layout_name 0: stereo
#tb 1: 1/90000
#media_type 1: video
-#codec_id 1: mjpeg
+#codec_id 1: bmp
#dimensions 1: 263x263
-#sar 1: 96/96
+#sar 1: 0/1
#tb 2: 1/90000
#media_type 2: video
-#codec_id 2: bmp
+#codec_id 2: mjpeg
#dimensions 2: 263x263
-#sar 2: 0/1
+#sar 2: 96/96
#tb 3: 1/90000
#media_type 3: video
#codec_id 3: png
#dimensions 3: 263x263
#sar 3: 1/1
0, -353590, -353590, 368640, 417, 0x15848290, S=1, 10
-1, 0, 0, 0, 15760, 0x71d5c418
-2, 0, 0, 0, 208350, 0x291b44d1
+1, 0, 0, 0, 208350, 0x291b44d1
+2, 0, 0, 0, 15760, 0x71d5c418
3, 0, 0, 0, 165671, 0x7c1c8070
0, 15050, 15050, 368640, 418, 0x46f684a4
0, 383690, 383690, 368640, 418, 0x46f684a4
@@ -36,15 +36,15 @@ TAG:encoder=Lavf
[/STREAM]
[STREAM]
index=1
-codec_name=mjpeg
+codec_name=bmp
DISPOSITION:attached_pic=1
-TAG:comment=Other
+TAG:comment=Band/Orchestra
[/STREAM]
[STREAM]
index=2
-codec_name=bmp
+codec_name=mjpeg
DISPOSITION:attached_pic=1
-TAG:comment=Band/Orchestra
+TAG:comment=Other
[/STREAM]
[STREAM]
index=3