[FFmpeg-devel] [PATCH] Fix gif decoder max option

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> James Almer
> Sent: Tuesday, September 17, 2019 3:29 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] Fix gif decoder max option
> 
> On 9/16/2019 10:24 PM, Soft Works wrote:
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >> James Almer
> >> Sent: Tuesday, September 17, 2019 3:11 AM
> >> To: ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] Fix gif decoder max option
> >>
> >> On 9/16/2019 10:05 PM, Soft Works wrote:
> >>> An int32 option cannot have a maximum of UINT32_MAX
> >>
> >> AV_OPT_TYPE_INT options are int64_t. In this case however the storage
> >> type for trans_color in GifState is int.
> >>
> >> Reading the code i see it's intended to be uint32_t, so i think the
> >> correct fix is changing its storage type, and not limiting its
> >> allowed range. Same with stored_bg_color.
> >
> > Hi James,
> >
> > Thanks for looking into this.
> >
> > The purpose of this option is to indicate a "replacement color" for
> transparent pixels.
> > Such a replacement color itself can never have an alpha component.
> >
> > I was unsure how to indicate this. Maybe, by setting the maximum to
> 0x00FFFFFF ?
> 
> In that case set the max to GIF_TRANSPARENT_COLOR (default value), which
> is 0x00FFFFFF.
> 
> Storage type as int in the struct is still wrong IMO, but i guess it's harmless. I'll
> let others chime in either way.

Thanks James,

I agree with all your comments. 
But to be honest, I didn't intend to get into gif decoder development.

I had just seen this:

$ ffmpeg -h decoder=gif

Decoder gif [GIF (Graphics Interchange Format)]:
    General capabilities: dr1
    Threading capabilities: none
gif decoder AVOptions:
  -trans_color       <int>        .D.V..... color value (ARGB) that is used instead of transparent color (from 0 to UINT32_MAX) (default 1.67772e+07)

Two things are incorrect here: The max and the default.

My patches are turning this into:

  -trans_color       <int>        .D.V..... color value (ARGB) that is used instead of transparent color (from 0 to INT_MAX) (default 16777215)

It might not be perfect, but it's much better than before.

Changing the storage type would need code changes to the gif decoder and require testing. At this time, I'm afraid, I'm not able to get into this. Of course, I'll do requested detail adjustments to my patches, though. :-)

Best regards,

softworkz