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

Michael Niedermayer michaelni at gmx.at
Fri Sep 19 13:48:40 CEST 2014


On Thu, Sep 18, 2014 at 10:34:11PM -0700, Pascal Massimino wrote:
> Hi Reimar,
> 
> On Thu, Sep 18, 2014 at 12:28 PM, Reimar Döffinger <Reimar.Doeffinger at gmx.de
> > wrote:
> 
> > 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.
> >
> >
> while i don't necessarily disagree with the above generally speaking [*],
> let me repeat that
> this is a pragmatic choice. I'm not going to give up on the speed-up i have
> right now for
> some hypothetical decoder later. This was facilitated by the fact that the
> change was minor
> and innocuous for ffmpeg.
> 
> /skal
> 
> (on a tangential side, this change also saves 2-6 bytes for files that
> actually have 0x00000000
> color in their palette, somewhat frequent case, because you then don't need
> to transmit this color)

should i apply the patch or wait ?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140919/575e7ce7/attachment.asc>


More information about the ffmpeg-devel mailing list