Opened 11 years ago

Closed 10 months ago

Last modified 10 months ago

#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 Elon Musk, 11 years ago

flac crc checks are mostly useless. flac could have error and crc check would just not report it.

comment:2 by Carl Eugen Hoyos, 11 years ago

Component: undeterminedavcodec
Keywords: flac, crc → flac crc
Resolution: fixed
Status: newclosed
Version: unspecifiedgit-master

Fixed by James Almer.

comment:3 by Balling, 10 months ago

Keywords: regression added
Resolution: fixed
Status: closedreopened

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 Elon Musk, 10 months ago

Resolution: fixed
Status: reopenedclosed

comment:5 by Elon Musk, 10 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 Elon Musk, 10 months ago

Keywords: regression removed

comment:7 by Balling, 10 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.

Last edited 10 months ago by Balling (previous) (diff)

comment:8 by Elon Musk, 10 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.

in reply to:  8 comment:9 by Balling, 10 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 Elon Musk, 10 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 Balling, 10 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 Elon Musk, 10 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

comment:13 by Balling, 10 months ago

#9621 sample flac file decode same MD5 hash as reference flac decoder.

As I said. "Was a valid file" and indeed I was talking about md5 being valid.

Opened: #10591

Last edited 10 months ago by Balling (previous) (diff)
Note: See TracTickets for help on using tickets.