PR #25250 opened by ben-xo
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25250
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25250.patch

# Summary of changes

Fixes trac ticket #10246: https://trac.ffmpeg.org/ticket/10246

AIFF files exported from Logic Pro 12.4 can end with three empty `ID3 ` chunks. 
`aiff_read_header()` passes the chunk size to `ff_id3v2_read()` as 
`max_search_size`, and 0 means no limit. The tag search then reads the next 
chunk header (`ID3 \0\0\0\0ID`) as an ID3v2.32 tag of 9412 bytes, which takes 
it past the end of the file. Depending on what follows, the file then fails to 
open ("Invalid data found when processing input", or "Input/output error" 
before ee623a43e3), or the chunks after the empty one are dropped.

The fix skips empty `ID3 ` chunks. The forward-only skip after 
`ff_id3v2_read()` is unchanged, because it keeps the handling of tags that are 
larger than their chunk.

The second commit adds `fate-aiff-empty-id3-demux`. Its sample is attached. It 
is synthetic: 50 ms of 1 kHz sine at 8 kHz written by FFmpeg as PCM S16BE AIFF, 
then three empty `ID3 ` chunks added and the FORM size corrected. The first 
commit does not depend on it.

Testing:
- The new test fails without the first commit and passes with it.
- `make fate` without samples passes (3048 tests), as do the AIFF and ID3 
sample tests `fate-id3v2-chapters`, `fate-id3v2-utf16-bom`, `fate-lavf-aiff`, 
`fate-seek-lavf-aiff` and the `adpcm-ima_qt` acodec and seek tests.
- We found this bug through AIFF uploads to Mixcloud that were exported from 
Logic Pro and failed to open. Those files now decode completely.

```fate-samples
aiff/empty_id3_chunks.aif
```



>From bcabf8379ba9bb1b567cb6eb79548773816e1c74 Mon Sep 17 00:00:00 2001
From: Ben XO <[email protected]>
Date: Tue, 20 Oct 2026 11:22:22 +0100
Subject: [PATCH 1/2] avformat/aiffdec: skip empty ID3 chunks

An ID3 chunk of size 0 was passed to ff_id3v2_read() as max_search_size
0, which means no limit. The tag search then read the bytes after the
empty chunk. Logic Pro writes three empty ID3 chunks at the end of every
AIFF, so the search read the next chunk header "ID3 \0\0\0\0ID" as an
ID3v2.32 tag of 9412 bytes and skipped past the end of the file. The
header parser then failed with "Invalid data found when processing input"
(EIO before ee623a43e3). With other chunks after the empty one, the
demuxer instead dropped those chunks, for example a NAME chunk.

An empty chunk holds no tag, so do not read one.

The forward-only skip after the read stays unchanged. It keeps the
handling of tags that are larger than their chunk.

Fixes ticket #10246.
---
 libavformat/aiffdec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c
index 63194f27263..bb4a2704779 100644
--- a/libavformat/aiffdec.c
+++ b/libavformat/aiffdec.c
@@ -277,6 +277,10 @@ static int aiff_read_header(AVFormatContext *s)
                 goto got_sound;
             break;
         case MKTAG('I', 'D', '3', ' '):
+            /* A max_search_size of 0 means no limit, so an empty chunk
+             * would make ff_id3v2_read() parse the chunks after it. */
+            if (!size)
+                break;
             position = avio_tell(pb);
             ff_id3v2_read(s, ID3v2_DEFAULT_MAGIC, &id3v2_extra_meta, size);
             if (id3v2_extra_meta)
-- 
2.54.0


>From 98f4ea880bf6d208d20e5a477cae528fb1edd3fa Mon Sep 17 00:00:00 2001
From: Ben XO <[email protected]>
Date: Tue, 20 Oct 2026 11:22:27 +0100
Subject: [PATCH 2/2] fate/demux: add test for AIFF with empty ID3 chunks

The sample is 50 ms of 1 kHz sine at 8 kHz, followed by three empty
ID3 chunks, as Logic Pro writes them. Without the previous commit the
demuxer fails to open it.

Sample: aiff/empty_id3_chunks.aif
---
 tests/fate/demux.mak                | 3 +++
 tests/ref/fate/aiff-empty-id3-demux | 6 ++++++
 2 files changed, 9 insertions(+)
 create mode 100644 tests/ref/fate/aiff-empty-id3-demux

diff --git a/tests/fate/demux.mak b/tests/fate/demux.mak
index d4ac666f810..d38175669cd 100644
--- a/tests/fate/demux.mak
+++ b/tests/fate/demux.mak
@@ -16,6 +16,9 @@ fate-aa-demux: CMD = framecrc -i $(TARGET_SAMPLES)/aa/bush.aa 
-c:a copy
 FATE_SAMPLES_DEMUX-$(call CRC, AEA) += fate-aea-demux
 fate-aea-demux: CMD = crc -i $(TARGET_SAMPLES)/aea/chirp.aea -c:a copy
 
+FATE_SAMPLES_DEMUX-$(call FRAMECRC, AIFF) += fate-aiff-empty-id3-demux
+fate-aiff-empty-id3-demux: CMD = framecrc -i 
$(TARGET_SAMPLES)/aiff/empty_id3_chunks.aif -c:a copy
+
 FATE_SAMPLES_DEMUX-$(call FRAMECRC, AV1, AV1, EXTRACT_EXTRADATA_BSF) += 
fate-av1-annexb-demux
 fate-av1-annexb-demux: CMD = framecrc -c:v av1 -i 
$(TARGET_SAMPLES)/av1/annexb.obu -c:v copy
 
diff --git a/tests/ref/fate/aiff-empty-id3-demux 
b/tests/ref/fate/aiff-empty-id3-demux
new file mode 100644
index 00000000000..b35825de7f0
--- /dev/null
+++ b/tests/ref/fate/aiff-empty-id3-demux
@@ -0,0 +1,6 @@
+#tb 0: 1/8000
+#media_type 0: audio
+#codec_id 0: pcm_s16be
+#sample_rate 0: 8000
+#channel_layout_name 0: mono
+0,          0,          0,      400,      800, 0x6f772b79
-- 
2.54.0

_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to