ffmpeg flac decoder incorrectly finds junk frame

[Mattias Wadman]

Summary of the bug:

Raw flac demuxer seems to incorrectly discard frames.

Example file was cut out from longer FLAC file that i unfortunately can't share. With the longer file the frames seem to also cause indeterministic behaviour when decoded using the ffmpeg http protocol.

Tested with master as of 2021-04-13 (33db0cbfd08384d611370006a77675cc6b778d12)

Looking at the offsets 0, 352, 486, 497, and 621 in the 4:the frame they all seems to be false headers found amoung nearly silence samples (lots of bits set) in the first subframe.

I was only able to produce a file triggering the issue using flac reference encoding.

How to reproduce:

# make && ./ffmpeg_g -v trace -f flac -i flac_junk_frame_issue.flac -f null -
ffmpeg version N-101944-g33db0cbfd0 Copyright (c) 2000-2021 the FFmpeg developers
  built with gcc 10.2.1 (Alpine 10.2.1_pre1) 20201203
  configuration: --enable-debug --disable-optimizations
  libavutil      56. 72.100 / 56. 72.100
  libavcodec     58.136.101 / 58.136.101
  libavformat    58. 78.100 / 58. 78.100
  libavdevice    58. 14.100 / 58. 14.100
  libavfilter     7.111.100 /  7.111.100
  libswscale      5. 10.100 /  5. 10.100
  libswresample   3. 10.100 /  3. 10.100
Splitting the commandline.
Reading option '-v' ... matched as option 'v' (set logging level) with argument 'trace'.
Reading option '-f' ... matched as option 'f' (force format) with argument 'flac'.
Reading option '-i' ... matched as input url with argument '/Users/wader/src/ffmpeg-master/flac_junk_frame_issue.flac'.
Reading option '-f' ... matched as option 'f' (force format) with argument 'null'.
Reading option '-' ... matched as output url.
Finished splitting the commandline.
Parsing a group of options: global .
Applying option v (set logging level) with argument trace.
Successfully parsed a group of options.
Parsing a group of options: input url /Users/wader/src/ffmpeg-master/flac_junk_frame_issue.flac.
Applying option f (force format) with argument flac.
Successfully parsed a group of options.
Opening an input file: /Users/wader/src/ffmpeg-master/flac_junk_frame_issue.flac.
[flac @ 0x7f03f3724040] Opening '/Users/wader/src/ffmpeg-master/flac_junk_frame_issue.flac' for reading
[file @ 0x7f03f37bc6c0] Setting default whitelist 'file,crypto,data'
[flac @ 0x7f03f3724040] Before avformat_find_stream_info() pos: 8304 bytes read:12029 seeks:0 nb_streams:1
[flac @ 0x7f03f37195c0] Junk frame till offset 714
[flac @ 0x7f03f37195c0] dropping low score 10 frame header from offset 0 to 352
[flac @ 0x7f03f37195c0] dropping low score -49 frame header from offset 352 to 486
[flac @ 0x7f03f37195c0] dropping low score -39 frame header from offset 486 to 497
[flac @ 0x7f03f37195c0] dropping low score -49 frame header from offset 497 to 623
[flac @ 0x7f03f37195c0] dropping low score -21 frame header from offset 623 to 714
[flac @ 0x7f03f3724040] All info found
[flac @ 0x7f03f3724040] stream 0: start_time: 0 duration: 0.5
[flac @ 0x7f03f3724040] format: start_time: 0 duration: 0.5 (estimate from stream) bitrate=192 kb/s
[flac @ 0x7f03f3724040] After avformat_find_stream_info() pos: 12029 bytes read:12029 seeks:0 frames:1
Input #0, flac, from '/Users/wader/src/ffmpeg-master/flac_junk_frame_issue.flac':
  Duration: 00:00:00.50, start: 0.000000, bitrate: 192 kb/s
  Stream #0:0, 1, 1/44100: Audio: flac, 44100 Hz, stereo, s16
Successfully opened the file.
Parsing a group of options: output url -.
Applying option f (force format) with argument null.
Successfully parsed a group of options.
Opening an output file: -.
Successfully opened the file.
detected 2 logical cores
Stream mapping:
  Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native))
Press [q] to stop, [?] for help
cur_dts is invalid st:0 (0) [init:0 i_done:0 finish:0] (this is harmless if it occurs once at the start per stream)
    Last message repeated 2 times
[graph_0_in_0_0 @ 0x7f03f36ff440] Setting 'time_base' to value '1/44100'
[graph_0_in_0_0 @ 0x7f03f36ff440] Setting 'sample_rate' to value '44100'
[graph_0_in_0_0 @ 0x7f03f36ff440] Setting 'sample_fmt' to value 's16'
[graph_0_in_0_0 @ 0x7f03f36ff440] Setting 'channel_layout' to value '0x3'
[graph_0_in_0_0 @ 0x7f03f36ff440] tb:1/44100 samplefmt:s16 samplerate:44100 chlayout:0x3
[format_out_0_0 @ 0x7f03f36ffe00] Setting 'sample_fmts' to value 's16'
[AVFilterGraph @ 0x7f03f371aa40] query_formats: 4 queried, 9 merged, 0 already done, 0 delayed
Output #0, null, to 'pipe:':
  Metadata:
    encoder         : Lavf58.78.100
  Stream #0:0, 0, 1/44100: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s
    Metadata:
      encoder         : Lavc58.136.101 pcm_s16le
[null @ 0x7f03f36fd040] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 20480 >= 16384
[null @ 0x7f03f36fd040] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 20480 >= 20480
[out_0_0 @ 0x7f03f36ff540] EOF on sink link out_0_0:default.
No more output streams to write to, finishing.
size=N/A time=00:00:00.55 bitrate=N/A speed=52.9x
video:0kB audio:86kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown
Input file #0 (/Users/wader/src/ffmpeg-master/flac_junk_frame_issue.flac):
  Input stream #0:0 (audio): 6 packets read (3725 bytes); 6 frames decoded (22050 samples);
  Total: 6 packets (3725 bytes) demuxed
Output file #0 (pipe:):
  Output stream #0:0 (audio): 6 frames encoded (22050 samples); 6 packets muxed (88200 bytes);
  Total: 6 packets (88200 bytes) muxed
6 frames successfully decoded, 0 decoding errors
[AVIOContext @ 0x7f03f3724c40] Statistics: 12029 bytes read, 0 seeks

[Carl Eugen Hoyos]

The file you attached is decoded correctly by FFmpeg, what do you want to report?

[Mattias Wadman]

Sorry, i should have made a better example file. Your right this one produced the exact same output as flac reference decoder. The new one is the same with some audio concatenated to it.

$ ffmpeg -i flac_junk_frame_issue2.flac flac_junk_frame_issue2.flac.ffmpeg.wav
...
$ flac -d -o flac_junk_frame_issue2.flac.ref.wav flac_junk_frame_issue2.flac
...
$ ffmpeg -i flac_junk_frame_issue2.flac.ffmpeg.wav -f s16le -ac 2 flac_junk_frame_issue2.flac.ffmpeg.wav.pcm
...
$ ffmpeg -i flac_junk_frame_issue2.flac.ref.wav -f s16le -ac 2 flac_junk_frame_issue2.flac.ref.wav.pcm
...

$ md5 *.pcm
MD5 (flac_junk_frame_issue2.flac.ffmpeg.wav.pcm) = a9ab411b9ab575be67693c03b56d621b
MD5 (flac_junk_frame_issue2.flac.ref.wav.pcm) = 901bdda002f27d63254d077d20985595

It looks like one silent flac frame (4096 samples) is missing.

[Mattias Wadman]

This made me also noticed that ffmpeg currently does not verify the samples md5.

Do you know if anyone has worked on adding that? think i might be interested in trying to add it. Had a quick look how the flac demuxer and decoder interact. I guess the md5 digest instance should be kept in FLACDecContext? but then should decoded samples be passed back to the demuxer or should the demuxer provide a md5 instance to the decoder? also verification would only work if no seeks were done.

Maybe should ask on -devel list?

[Carl Eugen Hoyos]

Replying to wader:

This made me also noticed that ffmpeg currently does not verify the samples md5.

To quote myself:
FFmpeg is not a verification tool for multimedia files.

Anyway:
Did you test with -err_detect +crccheck?

[Mattias Wadman]

-err_detect +crccheck does not seem to make any difference in output or logging.

Yes I understand ffmpeg primary tries to workaround and fix issues but in this case I don't think the file is broken, at least flac -t flac_junk_frame_issue2.flac does not find any errors.

Add optional samples md5 verification would both make it optionally possible to detect broken files and implementation bugs. As a note ffmpeg:s flac encoder/muuxer has support for calculating and writing samples md5.

[Mattias Wadman]

Looked a bit into this again. With this patch that only looks for frames with fixed blocking strategy flac_junk_frame_issue2.flac seems to decode correctly and produce samples that matches those produced by the FLAC reference decoder. So it seems as it ends up finding a false frame header somehow.

diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
index d3d9c889a1..c441543147 100644
--- a/libavcodec/flac_parser.c
+++ b/libavcodec/flac_parser.c
@@ -218,7 +218,7 @@ static int find_headers_search(FLACParseContext *fpc, uint8_t *buf,
         x = AV_RN32(buf + i);
         if (((x & ~(x + 0x01010101)) & 0x80808080)) {
             for (j = 0; j < 4; j++) {
-                if ((AV_RB16(buf + i + j) & 0xFFFE) == 0xFFF8) {
+                if ((AV_RB16(buf + i + j) & 0xFFFF) == 0xFFF8) {
                     int ret = find_headers_search_validate(fpc, search_start + i + j);
                     size = FFMAX(size, ret);
                 }

[Carl Eugen Hoyos]

Regression since 9300de0409d52272387a9b9d84143dba212291f4, work-around is to use -threads 1.

[Mattias Wadman]

-threads 1 makes flac_junk_frame_issue2.flac work for me also. I wonder if i'm seeing more than one issue as I managed cut out a small reproduction case for another flac file with similar error and warnings. In that case using -threads 1 does not seem to fix the issue.

Attaching an additional reproduction file

/build # flac -t flac_junk_frame_issue3.flac

flac 1.3.3
Copyright (C) 2000-2009  Josh Coalson, 2011-2016  Xiph.Org Foundation
flac comes with ABSOLUTELY NO WARRANTY.  This is free software, and you are
welcome to redistribute it under certain conditions.  Type `flac' for details.

flac_junk_frame_issue3.flac: ok


/build # ./ffmpeg_g -threads 1 -v trace -i flac_junk_frame_issue3.flac -f null -
ffmpeg version N-102608-g7a879cce37 Copyright (c) 2000-2021 the FFmpeg developers
  built with gcc 10.2.1 (Alpine 10.2.1_pre1) 20201203
  configuration: --enable-debug --disable-optimizations
  libavutil      57.  0.100 / 57.  0.100
  libavcodec     59.  1.100 / 59.  1.100
  libavformat    59.  2.101 / 59.  2.101
  libavdevice    59.  0.100 / 59.  0.100
  libavfilter     8.  0.101 /  8.  0.101
  libswscale      6.  0.100 /  6.  0.100
  libswresample   4.  0.100 /  4.  0.100
Splitting the commandline.
Reading option '-threads' ... matched as AVOption 'threads' with argument '1'.
Reading option '-v' ... matched as option 'v' (set logging level) with argument 'trace'.
Reading option '-i' ... matched as input url with argument 'flac_junk_frame_issue3.flac'.
Reading option '-f' ... matched as option 'f' (force format) with argument 'null'.
Reading option '-' ... matched as output url.
Finished splitting the commandline.
Parsing a group of options: global .
Applying option v (set logging level) with argument trace.
Successfully parsed a group of options.
Parsing a group of options: input url flac_junk_frame_issue3.flac.
Successfully parsed a group of options.
Opening an input file: flac_junk_frame_issue3.flac.
[NULL @ 0x7f9697dbb040] Opening 'flac_junk_frame_issue3.flac' for reading
[file @ 0x7f9697e53500] Setting default whitelist 'file,crypto,data'
Probing flac score:100 size:2048
[flac @ 0x7f9697dbb040] Format flac probed with size=2048 and score=100
[flac @ 0x7f9697dbb040] Before avformat_find_stream_info() pos: 8286 bytes read:11674 seeks:0 nb_streams:1
[flac @ 0x7f9697dba040] Junk frame till offset 3158
[flac @ 0x7f9697dba040] Inconsistent channel configuration.
[flac @ 0x7f9697dba040] get_buffer() failed
[flac @ 0x7f9697dba040] thread_get_buffer() failed
[flac @ 0x7f9697dbb040] stream 0: start_time: NOPTS duration: 0.0980045
[flac @ 0x7f9697dbb040] format: start_time: NOPTS duration: 0.098005 (estimate from stream) bitrate=952 kb/s
[flac @ 0x7f9697dbb040] After avformat_find_stream_info() pos: 11674 bytes read:11674 seeks:0 frames:3
Input #0, flac, from 'flac_junk_frame_issue3.flac':
  Metadata:
    encoder         : Lavf59.2.101
  Duration: 00:00:00.10, bitrate: 952 kb/s
  Stream #0:0, 3, 1/44100: Audio: flac, 44100 Hz, 2 channels (FL+FR+BL+BR), s32 (24 bit)
Successfully opened the file.
Parsing a group of options: output url -.
Applying option f (force format) with argument null.
Successfully parsed a group of options.
Opening an output file: -.
Successfully opened the file.
Stream mapping:
  Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native))
Press [q] to stop, [?] for help
cur_dts is invalid st:0 (0) [init:0 i_done:0 finish:0] (this is harmless if it occurs once at the start per stream)
detected 2 logical cores
[graph_0_in_0_0 @ 0x7f9697db3500] Setting 'time_base' to value '1/44100'
[graph_0_in_0_0 @ 0x7f9697db3500] Setting 'sample_rate' to value '44100'
[graph_0_in_0_0 @ 0x7f9697db3500] Setting 'sample_fmt' to value 's32'
[graph_0_in_0_0 @ 0x7f9697db3500] Setting 'channel_layout' to value '0x3'
[graph_0_in_0_0 @ 0x7f9697db3500] tb:1/44100 samplefmt:s32 samplerate:44100 chlayout:0x3
[format_out_0_0 @ 0x7f9697db3940] Setting 'sample_fmts' to value 's16'
[format_out_0_0 @ 0x7f9697db3940] auto-inserting filter 'auto_resampler_0' between the filter 'Parsed_anull_0' and the filter 'format_out_0_0'
[AVFilterGraph @ 0x7f9697db4680] query_formats: 4 queried, 6 merged, 3 already done, 0 delayed
[auto_resampler_0 @ 0x7f9697db2c00] [SWR @ 0x7f9697d39040] Using fltp internally between filters
[auto_resampler_0 @ 0x7f9697db2c00] ch:2 chl:stereo fmt:s32 r:44100Hz -> ch:2 chl:stereo fmt:s16 r:44100Hz
Output #0, null, to 'pipe:':
  Metadata:
    encoder         : Lavf59.2.101
  Stream #0:0, 0, 1/44100: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s
    Metadata:
      encoder         : Lavc59.1.100 pcm_s16le
[flac @ 0x7f9697db9040] invalid residualeed=1.23e+04x
[flac @ 0x7f9697db9040] decode_frame() failed
Error while decoding stream #0:0: Invalid data found when processing input
[flac @ 0x7f9697db9040] switching bps mid-stream is not supported
[flac @ 0x7f9697db9040] decode_frame() failed
Error while decoding stream #0:0: Invalid data found when processing input
[out_0_0 @ 0x7f9697db3840] EOF on sink link out_0_0:default.
No more output streams to write to, finishing.
size=N/A time=00:00:00.09 bitrate=N/A speed=40.4x
video:0kB audio:17kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown
Input file #0 (flac_junk_frame_issue3.flac):
  Input stream #0:0 (audio): 3 packets read (6776 bytes); 1 frames decoded (4322 samples);
  Total: 3 packets (6776 bytes) demuxed
Output file #0 (pipe:):
  Output stream #0:0 (audio): 1 frames encoded (4322 samples); 1 packets muxed (17288 bytes);
  Total: 1 packets (17288 bytes) muxed
1 frames successfully decoded, 2 decoding errors
[AVIOContext @ 0x7f9697dbb840] Statistics: 11674 bytes read, 0 seeks

[Mattias Wadman]

Maybe interesting note, the flac file was written by ffmpeg

$ flac -o flac_junk_frame_issue3.flac.wav flac_junk_frame_issue3.flac
$ ffmpeg -i flac_junk_frame_issue3.flac.wav flac_junk_frame_issue3.flac.wav.flac
$ flac -t
...
flac_junk_frame_issue3.flac.wav.flac: ok
$ ffmpeg -i flac_junk_frame_issue3.flac.wav.flac -f null -
...
[flac @ 0x7ff4f6ad9c00] Junk frame till offset 3158
[flac @ 0x7f3d89d63c00] Inconsistent channel configuration.
[flac @ 0x7f3d89d63c00] get_buffer() failed
[flac @ 0x7f3d89d63c00] thread_get_buffer() failed
...
[flac @ 0x7ff4f6ace040] switching bps mid-stream is not supported
[flac @ 0x7ff4f6ace040] decode_frame() failed
[flac @ 0x7ff4f6ad0800] invalid residual
[flac @ 0x7ff4f6ad0800] decode_frame() failed
...

The offset 3158 into first (and only) frame in the example file is the sample data for the second partition of the second subframe and it has multiple occurrences of the bit pattern 0xfff8.

[Mattias Wadman]

Hello, spent some more time on this as i keep seeing files with this issue from time to time.

Could a patch like this be ok to merge into ffmpeg? not perfect but fixes the issue for the files i've seen. It peeks one byte into the first subframe and looks for valid configurations.

diff --git a/libavcodec/flac_parser.c b/libavcodec/flac_parser.c
index 3424583c49..de9651926b 100644
--- a/libavcodec/flac_parser.c
+++ b/libavcodec/flac_parser.c
@@ -96,8 +96,34 @@ static int frame_header_is_valid(AVCodecContext *avctx, const uint8_t *buf,
                                  FLACFrameInfo *fi)
 {
     GetBitContext gb;
-    init_get_bits(&gb, buf, MAX_FRAME_HEADER_SIZE * 8);
-    return !ff_flac_decode_frame_header(avctx, &gb, fi, 127);
+    uint8_t subframe_type;
+
+    // header plus one byte from first subframe
+    init_get_bits(&gb, buf, MAX_FRAME_HEADER_SIZE * 8 + 8);
+    if (ff_flac_decode_frame_header(avctx, &gb, fi, 127) != 0) {
+        return 0;
+    }
+    // subframe zero bit
+    if (get_bits1(&gb) != 0) {
+        return 0;
+    }
+    // subframe subframe_type
+    // 000000 : SUBFRAME_CONSTANT
+    // 000001 : SUBFRAME_VERBATIM
+    // 00001x : reserved
+    // 0001xx : reserved
+    // 001xxx : if(xxx <= 4) SUBFRAME_FIXED, xxx=order ; else reserved
+    // 01xxxx : reserved
+    // 1xxxxx : SUBFRAME_LPC, xxxxx=order-1
+    subframe_type = get_bits(&gb, 6);
+    if (!(subframe_type == 0 ||
+          subframe_type == 1 ||
+          ((subframe_type >= 8) && (subframe_type <= 12)) ||
+          (subframe_type >= 32))) {
+        return 0;
+    }
+
+    return 1;
 }

 /**

[Mattias Wadman]

Fixed for all cases that i know of by 49597300e87c5c4b2ca56c5b93930d92f64cdf5b but there is still a probability that the flac parse misidentify frames.

Close as fixed?

[Elon Musk]

Feel free to open new ticket if new issues arise.