[FFmpeg-devel] [PATCH]: libavcodec/webp

[Reimar Döffinger]

On 18 September 2014 10:55:00 CEST, Pascal Massimino <pascal.massimino at gmail.com> wrote:
>Hi Reimar,
>
>On Thu, Sep 18, 2014 at 9:33 AM, Reimar Döffinger
><Reimar.Doeffinger at gmx.de>
>wrote:
>
>> On 18.09.2014, at 08:45, Pascal Massimino
><pascal.massimino at gmail.com>
>> wrote:
>> > Hi,
>> >
>> > the webp lossless doc was unclear regarding palette index falling
>out of
>> > range.
>> > See issue #206 [1] for the bug report.
>> > The solution retained was to treat out-of-range index as color
>> 0x00000000,
>> > so we could keep the speed-up in libwebp (we use fake extra entries
>in
>> the
>> > cmap to handle out-of-bound values without the extra branch).
>> >
>> > The doc was clarified (along with few other loose ends) in patch
>#71605
>> [2]
>> > Attached is the fix for ffmpeg's webp decoder to treat out-of-bound
>index
>> > as transparent-black instead of reporting an error.
>>
>> The spec now says "should be set". I don't fully understand why we
>would
>> ordinarily expect out-of-range value.
>>
>
>corrupt files, for instance. Or just malicious ones.

Why in all the world would you make handling of corrupted and malicious files part of the spec?
That certainly should be left up to the decoder!
A safety-oriented or conformance checking one would immediately abort, a speed oriented one would do whatever is fastest and a data recovery one would use advanced error resilience methods.
Why would it be desirable for all of them to be forced to do the same thing?

>I also don't understand why the issue talks about speed loss in the
>> decoder, when it is the encoder that decides to use out-of-bounds
>values.
>>
>
>I considered both options: making the values forbidden, or defining a
>behaviour for out-of-bound.
>It turned out (see bug issue text) that introducing the bound-check in
>an
>already tight-loop was slowing down decoding up to 9%.
>All things considered, pragmatism indicates that the latter option
>should
>be favored.

Sorry that makes no sense.
If an input is invalid you decoder can do whatever it wants.
Declaring something as invalid in the spec purely logically can in no way cause a slowdown.
Now for libwebp you might have some issues if you insist that you want it to be both reference decoder and fast at the same time. But that's an issue with your choice of implementing things, not the spec.
With the explanation you gave, I would say both your change to the spec and the proposed FFmpeg change are just nonsense.