[FFmpeg-devel] [PATCH] pngdec: add ability to check chunk CRC

[Lynne]

8 Mar 2019, 14:15 by dev at lynne.ee:

> 7 Mar 2019, 21:22 by > michael at niedermayer.cc <mailto:michael at niedermayer.cc>> :
>
>> On Thu, Mar 07, 2019 at 07:26:32PM +0100, Lynne wrote:
>>
>>> By default now, if AV_EF_CRCCHECK or AV_EF_IGNORE_ERR are enabled the decoder
>>> will skip the chunk and carry on with the next one. This should make the       
>>> decoder able to decode more corrupt files because the functions which decode
>>> individual chunks will very likely error out if fed invalid data and stop the
>>> decoding of the entire image.
>>> Should this be made default? CRC verification doesn't take long even for very
>>> large files.                                                      
>>> Also fix the length check for chunk size. It needs to take into account the
>>> 4 byte tag as well as the 4 byte CRC.
>>>
>>> pngdec.c |   19 ++++++++++++++++++-
>>>  1 file changed, 18 insertions(+), 1 deletion(-)
>>> 4255c91468cee2bc2fa757fae69762ff5ee5774a  0001-pngdec-add-ability-to-check-chunk-CRC.patch
>>> From 7aff99d12faf557753c5ee860a9672c7a09a26e3 Mon Sep 17 00:00:00 2001
>>> From: Lynne <>> >>> dev at lynne.ee <mailto:dev at lynne.ee>>>>  <mailto:>>> dev at lynne.ee <mailto:dev at lynne.ee>>>> >>> >
>>> Date: Thu, 7 Mar 2019 18:15:23 +0000
>>> Subject: [PATCH] pngdec: add ability to check chunk CRC
>>>
>>> By default now, if AV_EF_CRCCHECK or AV_EF_IGNORE_ERR are enabled the decoder
>>> will skip the chunk and carry on with the next one. This should make the
>>> decoder able to decode more corrupt files because the functions which decode
>>> individual chunks will very likely error out if fed invalid data and stop the
>>> decoding of the entire image.
>>> Should this be made default? CRC verification doesn't take long even for very
>>> large files.
>>>
>>
>> i would tend toward enabling it by default but maybe first post some
>> numbers of how much this changes decode time 
>>
>
> For the largest png I found: > https://vk.com/doc218587497_437472325?hash=51300ca9ba40f462ac&dl=1bcf9a57b0d989da1f <https://vk.com/doc218587497_437472325?hash=51300ca9ba40f462ac&dl=1bcf9a57b0d989da1f>>  <> https://vk.com/doc218587497_437472325?hash=51300ca9ba40f462ac&dl=1bcf9a57b0d989da1f <https://vk.com/doc218587497_437472325?hash=51300ca9ba40f462ac&dl=1bcf9a57b0d989da1f>> >
>
> There was no increase in decoding time, it took 2.3 seconds on my machine
> with and without -err_detect crccheck.
>
> With -err_detect crccheck perf reported 2.52% spent in av_crc
>
>
>>> Also fix the length check for chunk size. It needs to take into account the
>>> 4 byte tag as well as the 4 byte CRC.
>>>
>>
>> this should be a seperate patch as its unrelated
>>
>
> removed and attached new patch file
>
> Maybe always enabling the CRC check isn't worth it since if you download from
> the internet you could either get a full error-free file or an incomplete one
> rather than a corrupt one. Maybe only for torrents with huge png files where
> not all chunks have been downloaded yet, or broken hard drives, but the ignore_err
> flag could be manually enabled in those cases.
>

Ping? The apedec CRC patch needs a review too.