[FFmpeg-devel] [PATCH] avcodec/flac: check frame header crc only if requested

[dave at dericed.com]

> On Dec 8, 2016, at 12:45 PM, James Almer <jamrial at gmail.com> wrote:
> 
> On 12/8/2016 9:31 AM, Michael Niedermayer wrote:
>> FFmpeg decoders primary usecase is to decode for human consumption
>> for this producing the best quality possible and doing so fast is
>> the goal.
>> The default thus should be to do checks that improve quality
>> its probably better if fuzzers disable these checks instead of the
>> "default being changed"
> 
> This patch is needed for fuzzers to actually disable the check.
> I assume there's a high chance for fuzzers to change some bits on each
> frame header, and if the decoder aborts unconditionally right then and
> there then the fuzzing is unlikely to ever find any real bug in the code.
> 
> If a real file has a damaged frame header, then even if it doesn't abort
> at that point it will fail somewhere down the line anyway, due to a bogus
> channel count or framesize value.
> 
>> 
>> treating damaged headers as if they are valid will lead to bad
>> quality at the location where this occurs.
>> also this function is used by the parser and i _think_ the code expects
>> that it always checks the crc.
> 
> If this is true then this patch is not good.
> 
>> also for detecting the parameters of a stream, its important to use
>> a undamaged header otherwise the parameters could be more often wrong.
>> 
>> whatever default is used for err_recognition some checks like this
>> one should be enabled in that case
>> actually i thought we have a non zero default but it seems 0 in
>> libavcodec, i think this is bad as well.
>> With a 0 default nothing can be enabled by default (if the mask is all
>> positive with 0 being all disabled)
> 
> A couple years ago crccheck was the default value for err_recognition,
> but you sent a patch to disable it since it was affecting hevc decoding
> performance. I don't think that has changed since then.
> 
> Do we have a way to add codec dependent defaults for options like
> err_recognition? lossless codecs could then have crc enabled by default
> and lossy codecs have it disabled, for example.

+1

>> [...]
>> 
>> 
>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel