#2266 closed enhancement (fixed)
support flac crcchecks
Reported by: | dave rice | Owned by: | |
---|---|---|---|
Priority: | wish | Component: | avcodec |
Version: | git-master | Keywords: | flac crc |
Cc: | Blocked By: | ||
Blocking: | Reproduced by developer: | no | |
Analyzed by developer: | no |
Description
Summary of the bug:
FLAC uses an md5 in the header to verify that the audio data is correct. It also uses crcs to allow any damage to be identified more precisely. FFmpeg has a -err_detect crccheck option but it doesn't work on flac.
How to reproduce:
Apply a data fuzzer to a valid flac file.
ffmpeg -i test.flac -f null -err_detect crccheck - ffmpeg version 1.1.git Copyright (c) 2000-2013 the FFmpeg developers built on Feb 12 2013 19:07:29 with Apple clang version 4.1 (tags/Apple/clang-421.11.66) (based on LLVM 3.1svn) configuration: --prefix=/usr/local/Cellar/ffmpeg/HEAD --enable-shared --enable-pthreads --enable-gpl --enable-version3 --enable-nonfree --enable-hardcoded-tables --enable-avresample --cc=cc --host-cflags= --host-ldflags= --enable-libx264 --enable-libfaac --enable-libmp3lame --enable-libxvid --enable-ffplay --enable-libopenjpeg --extra-cflags='-I/usr/local/Cellar/openjpeg/1.5.1/include/openjpeg-1.5 ' libavutil 52. 17.101 / 52. 17.101 libavcodec 54. 91.103 / 54. 91.103 libavformat 54. 63.100 / 54. 63.100 libavdevice 54. 3.103 / 54. 3.103 libavfilter 3. 37.101 / 3. 37.101 libswscale 2. 2.100 / 2. 2.100 libswresample 0. 17.102 / 0. 17.102 libpostproc 52. 2.100 / 52. 2.100 [flac @ 0x7f932a836e00] max_analyze_duration 5000000 reached at 5088000 microseconds Input #0, flac, from 'test.flac': Metadata: MAJOR_BRAND : isom MINOR_VERSION : 1 COMPATIBLE_BRANDS: isom ENCODER : Lavf54.63.100 Duration: 00:00:30.05, bitrate: 996 kb/s Stream #0:0: Audio: flac, 48000 Hz, stereo, s32 Output #0, null, to 'pipe:': Metadata: MAJOR_BRAND : isom MINOR_VERSION : 1 COMPATIBLE_BRANDS: isom encoder : Lavf54.63.100 Stream #0:0: Audio: pcm_s16le, 48000 Hz, stereo, s16, 1536 kb/s Stream mapping: Stream #0:0 -> #0:0 (flac -> pcm_s16le) Press [q] to stop, [?] for help size=N/A time=00:00:30.04 bitrate=N/A video:0kB audio:5634kB subtitle:0 global headers:0kB muxing overhead -100.000381%
No error is provided, although the file is damaged with a crc mismatch.
Via the flac utility the same file is assessed like this:
flac -d -V test.flac flac 1.2.1, Copyright (C) 2000,2001,2002,2003,2004,2005,2006,2007 Josh Coalson flac comes with ABSOLUTELY NO WARRANTY. This is free software, and you are welcome to redistribute it under certain conditions. Type `flac' for details. test.flac: 20% completetest.flac: *** Got error code 2:FLAC__STREAM_DECODER_ERROR_STATUS_FRAME_CRC_MISMATCH test.flac: ERROR while decoding data state = FLAC__STREAM_DECODER_READ_FRAME
Although flac acknowledges the presence of an error it only notes that it occurs ~20% into the decoding. I would recommend ffmpeg report the pts of where the error is identified, similar to how the crccheck is reported in ffv1.3.
Change History (13)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Component: | undetermined → avcodec |
---|---|
Keywords: | flac, crc → flac crc |
Resolution: | → fixed |
Status: | new → closed |
Version: | unspecified → git-master |
Fixed by James Almer.
comment:3 by , 15 months ago
Keywords: | regression added |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
This is now broken again. ffmpeg.exe -err_detect crccheck -i kvvrac.flac -f null -
Should print [flac @ 000002304147a380] CRC error at PTS 29583360
File: https://files.catbox.moe/kvvrac.flac
comment:4 by , 15 months ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
comment:5 by , 15 months ago
Open new ticket if you think that is really broken and not some ad-hoc your thinking that is broken.
comment:6 by , 15 months ago
Keywords: | regression removed |
---|
comment:7 by , 15 months ago
It clearly regressed. Where would I get the CRC error at PTS 29583360 otherwise? I used an old version.
flac crc checks are mostly useless. flac could have error and crc check would just not report it.
Well, it is not useless in this case.
follow-up: 9 comment:8 by , 15 months ago
FLAC parser is more robust now, if you enable -v debug you will see that FLAC decoder have actual underreads, it discards data that is not valid.
comment:9 by , 15 months ago
Replying to Elon Musk:
FLAC parser is more robust now, if you enable -v debug you will see that FLAC decoder have actual underreads, it discards data that is not valid.
Actually, the wav file you get from https://files.catbox.moe/kvvrac.flac was bigger back when crcchecks worked. But anyway, it would be nice to add crcchecks back.
comment:10 by , 15 months ago
Parser is doing extra crc checks internally that is hidden from user eyes, more than before. Before you would get sometimes invalid data from fully valid .flac files.
The underread debug could be made a warning. And for bigger file that is because it would fill it with silence or pure noise, better to just trim it like now.
comment:11 by , 15 months ago
Before you would get sometimes invalid data from fully valid .flac files.
#9621 was valid file that flac.exe accepts no problem, but you broke this issue, I even mentioned
I think we are not going to validate crc16 by default
So not by default we still will. Just like this "verified with the AV_EF_CRCCHECK flag": https://patchwork.ffmpeg.org/project/ffmpeg/patch/20230809174657.76-1-tcChlisop0@gmail.com/
By the way, I have a question. https://files.catbox.moe/kvvrac.flac is a file from torrents (you can find it with Btdigg). Can you tell me why is it corrupted? Is it possible to bruteforce and restore it?
Also, I do not get it why cannot I reopen old issues. It is common to do that when issue regresses in other projects.
comment:12 by , 15 months ago
#9621 sample flac file decode same MD5 hash as reference flac decoder.
If you have extraordinary proof for extraordinary claim than by all means open new ticket.
If you want consulting help, visit http://ffmpeg.org/consulting.html
flac crc checks are mostly useless. flac could have error and crc check would just not report it.