Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Paul B Mahol]

Hi,

On 11/27/17, Carl Eugen Hoyos  wrote:
> 2017-11-27 18:50 GMT+01:00 Bjorn Roche :
>> Sorry for the very delayed response, Carl:
>>
>> On Tue, Nov 21, 2017 at 6:55 PM, Carl Eugen Hoyos 
>> wrote:
>>
>>> 2017-11-21 18:53 GMT+01:00 Bjorn Roche :
>>> > Support for transparencies in animated gifs requires modifying both
>>> > libavcodec/gif.c and libavformat/gif.c because both the graphics
>>> > control extension (handled by libavformat/gif.c) and the raw frame data
>>> > (handled by libavcodec/gif.c) must be changed.
>>>
>>> Does remuxing animated gif with transparency work with your patch?
>>
>> A command like this:
>>
>> ffmpeg -i in.gif out.gif
>>
>> seems to produce the same output as before: transparency replaces
>> with opacity.
>
> remuxing == -vcodec copy
>
> What about ffmpeg -i in.gif -vcodec copy out.gif ?
>
>> However a command like this:
>>
>> ffmpeg -i in.gif  -vf palettegen -y /tmp/pal.png && ffmpeg -i in.gif
>> -i /tmp/pal.png -lavfi paletteuse -y -f gif out.gif
>>
>> produces an output that looks like the input (including transparency).
>>
>> I believe this is a problem with the decoder, however, since
>>
>> ffmpeg -i /Users/bjorn/Desktop/smoketrail.gif
>> /Users/bjorn/Desktop/smoketrail-out%d.png
>
> This produces transparent images here for the output
> file of ticket #6813.

Can anybody get this patch to output correct output for this command:
ffmpeg -v debug -f lavfi -i testsrc2=d=10:alpha=0,format=rgba -lavfi
"[0:v]palettegen[p],[0:v][p]paletteuse" 1.gif ?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Paul B Mahol]

Hi,,

On 11/21/17, Bjorn Roche  wrote:
> Support for transparencies in animated gifs requires modifying both
> libavcodec/gif.c and libavformat/gif.c because both the graphics
> control extension (handled by libavformat/gif.c) and the raw frame data
> (handled by libavcodec/gif.c) must be changed. This is because
> transparencies in GIF can be used both to create a transparent image,
> and to provide optimization.
>
> How transparencies are interpreted in a given frame is controlled by
> the “disposal method”, which must be set appropriately in the graphics
> control extension.
>
>  The “in place” disposal method is used when transparency indicates
> optimization, and the “background” disposal method is used when
> transparency is intended to be preserved. In order to support both
> disposal methods, libavcodec/gif.c must signal to libavformat/gif.c
> which disposal method is required for every frame. This is done with a
> new side data type: AV_PKT_DATA_GIF_FRAME_DISPOSAL. This requires a
> change to avcodec.h
>
> Unfortunately, the addition of a new side data type causes some of the
> FATE tests to fail, so the fate tests are updated here as well.

What about rewritting code instead so it behaves correctly?

For example muxing gif and encoding gif should work with codec copy.
To get that working one needs to change muxer and encoder so muxer does not
write extensions but encoder does.
Adding side data is not going to make codec copy work.

I'm planning to do this.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Carl Eugen Hoyos]

2018-01-16 19:40 GMT+01:00 Bjorn Roche :

> I'm just reviewing this and noticing that the patch is stale
> and no longer compiles. I am happy to update, but is
> there someone to review it if I do so?

This is Clément's code, he has to review your patch.

In any case, please update the patch.

Sorry, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Bjorn Roche]

Hey all,

I'm just reviewing this and noticing that the patch is stale and no longer
compiles. I am happy to update, but is there someone to review it if I do
so? I'd love to get this patch in. Thanks,

bjorn


On Thu, Dec 7, 2017 at 5:56 PM, Bjorn Roche  wrote:

> Hey all,
>
> Just wondering what else is needed to get this merged? Is there anything
> else I can do?
>
> Thanks,
>
> bjorn
>
> On Thu, Nov 30, 2017 at 12:33 PM, Bjorn Roche  wrote:
>
>> We've done a bit more testing on this and it looks good here.
>>
>> On Thu, Nov 30, 2017 at 12:26 PM, Bjorn Roche  wrote:
>>
>>>
>>>
>>> On Mon, Nov 27, 2017 at 7:12 PM, Carl Eugen Hoyos 
>>> wrote:
>>>
 2017-11-28 1:07 GMT+01:00 Bjorn Roche :
 > On Mon, Nov 27, 2017 at 5:41 PM, Carl Eugen Hoyos 
 > wrote:

 >> >> Does remuxing animated gif with transparency work with your patch?
 >> >
 >> > A command like this:
 >> >
 >> > ffmpeg -i in.gif out.gif
 >> >
 >> > seems to produce the same output as before: transparency replaces
 >> > with opacity.
 >>
 >> remuxing == -vcodec copy
 >>
 >> What about ffmpeg -i in.gif -vcodec copy out.gif ?
 >>
 >
 > That doesn't work. Weather I use my patch or not it aborts:

 Of course, ticket #6640.
>>>
>>>
>>> I'm going to see if this is something I can tackle separately.
>>>
>>> bjorn
>>>
>>>
>>> --
>>>
>>>
>>> Bjorn Roche
>>>
>>> Sr. Video Pipeline Engineer
>>>
>>> bj...@giphy.com
>>>
>>
>>
>>
>> --
>>
>>
>> Bjorn Roche
>>
>> Sr. Video Pipeline Engineer
>>
>> bj...@giphy.com
>>
>
>
>
> --
>
>
> Bjorn Roche
>
> Sr. Video Pipeline Engineer
>
> bj...@giphy.com
>



-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Bjorn Roche]

Hey all,

Just wondering what else is needed to get this merged? Is there anything
else I can do?

Thanks,

bjorn

On Thu, Nov 30, 2017 at 12:33 PM, Bjorn Roche  wrote:

> We've done a bit more testing on this and it looks good here.
>
> On Thu, Nov 30, 2017 at 12:26 PM, Bjorn Roche  wrote:
>
>>
>>
>> On Mon, Nov 27, 2017 at 7:12 PM, Carl Eugen Hoyos 
>> wrote:
>>
>>> 2017-11-28 1:07 GMT+01:00 Bjorn Roche :
>>> > On Mon, Nov 27, 2017 at 5:41 PM, Carl Eugen Hoyos 
>>> > wrote:
>>>
>>> >> >> Does remuxing animated gif with transparency work with your patch?
>>> >> >
>>> >> > A command like this:
>>> >> >
>>> >> > ffmpeg -i in.gif out.gif
>>> >> >
>>> >> > seems to produce the same output as before: transparency replaces
>>> >> > with opacity.
>>> >>
>>> >> remuxing == -vcodec copy
>>> >>
>>> >> What about ffmpeg -i in.gif -vcodec copy out.gif ?
>>> >>
>>> >
>>> > That doesn't work. Weather I use my patch or not it aborts:
>>>
>>> Of course, ticket #6640.
>>
>>
>> I'm going to see if this is something I can tackle separately.
>>
>> bjorn
>>
>>
>> --
>>
>>
>> Bjorn Roche
>>
>> Sr. Video Pipeline Engineer
>>
>> bj...@giphy.com
>>
>
>
>
> --
>
>
> Bjorn Roche
>
> Sr. Video Pipeline Engineer
>
> bj...@giphy.com
>



-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Bjorn Roche]

We've done a bit more testing on this and it looks good here.

On Thu, Nov 30, 2017 at 12:26 PM, Bjorn Roche  wrote:

>
>
> On Mon, Nov 27, 2017 at 7:12 PM, Carl Eugen Hoyos 
> wrote:
>
>> 2017-11-28 1:07 GMT+01:00 Bjorn Roche :
>> > On Mon, Nov 27, 2017 at 5:41 PM, Carl Eugen Hoyos 
>> > wrote:
>>
>> >> >> Does remuxing animated gif with transparency work with your patch?
>> >> >
>> >> > A command like this:
>> >> >
>> >> > ffmpeg -i in.gif out.gif
>> >> >
>> >> > seems to produce the same output as before: transparency replaces
>> >> > with opacity.
>> >>
>> >> remuxing == -vcodec copy
>> >>
>> >> What about ffmpeg -i in.gif -vcodec copy out.gif ?
>> >>
>> >
>> > That doesn't work. Weather I use my patch or not it aborts:
>>
>> Of course, ticket #6640.
>
>
> I'm going to see if this is something I can tackle separately.
>
> bjorn
>
>
> --
>
>
> Bjorn Roche
>
> Sr. Video Pipeline Engineer
>
> bj...@giphy.com
>



-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Bjorn Roche]

On Mon, Nov 27, 2017 at 7:12 PM, Carl Eugen Hoyos 
wrote:

> 2017-11-28 1:07 GMT+01:00 Bjorn Roche :
> > On Mon, Nov 27, 2017 at 5:41 PM, Carl Eugen Hoyos 
> > wrote:
>
> >> >> Does remuxing animated gif with transparency work with your patch?
> >> >
> >> > A command like this:
> >> >
> >> > ffmpeg -i in.gif out.gif
> >> >
> >> > seems to produce the same output as before: transparency replaces
> >> > with opacity.
> >>
> >> remuxing == -vcodec copy
> >>
> >> What about ffmpeg -i in.gif -vcodec copy out.gif ?
> >>
> >
> > That doesn't work. Weather I use my patch or not it aborts:
>
> Of course, ticket #6640.


I'm going to see if this is something I can tackle separately.

bjorn


-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Carl Eugen Hoyos]

2017-11-28 1:12 GMT+01:00 Bjorn Roche :
> On Mon, Nov 27, 2017 at 5:35 PM, Carl Eugen Hoyos 
> wrote:
>
>> 2017-11-27 19:30 GMT+01:00 Bjorn Roche :
>> >
>>
>> > The only way I can see is to move some functionality between the mixer
>> and
>> > the encoder. It's not a clear line between the codec and the container in
>> > GIF, so I wouldn't object to an approach different from the one I took.
>>
>> (Just a question!)
>>
>> Does remuxing (animated) gif to gif work now? (I believe it does.)
>> Does it work with your patch?
>
> I think this would require changes to the reader. AFAICT, it only outputs
> BGRA.

> Something I can look into if that seems right to you.

I have no idea.
I just wanted to point out some tests that may or may not
show if the whole approach makes sense.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Bjorn Roche]

On Mon, Nov 27, 2017 at 5:35 PM, Carl Eugen Hoyos 
wrote:

> 2017-11-27 19:30 GMT+01:00 Bjorn Roche :
> >
>
> > The only way I can see is to move some functionality between the mixer
> and
> > the encoder. It's not a clear line between the codec and the container in
> > GIF, so I wouldn't object to an approach different from the one I took.
>
> (Just a question!)
>
> Does remuxing (animated) gif to gif work now? (I believe it does.)
> Does it work with your patch?


I think this would require changes to the reader. AFAICT, it only outputs
BGRA. Something I can look into if that seems right to you.


-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Carl Eugen Hoyos]

2017-11-28 1:07 GMT+01:00 Bjorn Roche :
> On Mon, Nov 27, 2017 at 5:41 PM, Carl Eugen Hoyos 
> wrote:

>> >> Does remuxing animated gif with transparency work with your patch?
>> >
>> > A command like this:
>> >
>> > ffmpeg -i in.gif out.gif
>> >
>> > seems to produce the same output as before: transparency replaces
>> > with opacity.
>>
>> remuxing == -vcodec copy
>>
>> What about ffmpeg -i in.gif -vcodec copy out.gif ?
>>
>
> That doesn't work. Weather I use my patch or not it aborts:

Of course, ticket #6640.

Sorry, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Bjorn Roche]

On Mon, Nov 27, 2017 at 5:41 PM, Carl Eugen Hoyos 
wrote:

> 2017-11-27 18:50 GMT+01:00 Bjorn Roche :
> > Sorry for the very delayed response, Carl:
> >
> > On Tue, Nov 21, 2017 at 6:55 PM, Carl Eugen Hoyos 
> > wrote:
> >
> >> 2017-11-21 18:53 GMT+01:00 Bjorn Roche :
> >> > Support for transparencies in animated gifs requires modifying both
> >> > libavcodec/gif.c and libavformat/gif.c because both the graphics
> >> > control extension (handled by libavformat/gif.c) and the raw frame
> data
> >> > (handled by libavcodec/gif.c) must be changed.
> >>
> >> Does remuxing animated gif with transparency work with your patch?
> >
> > A command like this:
> >
> > ffmpeg -i in.gif out.gif
> >
> > seems to produce the same output as before: transparency replaces
> > with opacity.
>
> remuxing == -vcodec copy
>
> What about ffmpeg -i in.gif -vcodec copy out.gif ?
>

That doesn't work. Weather I use my patch or not it aborts:

Assertion video_par->format == AV_PIX_FMT_PAL8 failed at
libavformat/gif.c:132

As far as I can tell, the gif decoder (which I haven't touched) outputs
BGRA, but the gif writer requires PAL8.

> However a command like this:
> >
> > ffmpeg -i in.gif  -vf palettegen -y /tmp/pal.png && ffmpeg -i in.gif
> > -i /tmp/pal.png -lavfi paletteuse -y -f gif out.gif
> >
> > produces an output that looks like the input (including transparency).
> >
> > I believe this is a problem with the decoder, however, since
> >
> > ffmpeg -i /Users/bjorn/Desktop/smoketrail.gif
> > /Users/bjorn/Desktop/smoketrail-out%d.png
>
> This produces transparent images here for the output
> file of ticket #6813.


./ffmpeg -i transparency.apng out.gif

Produces the expected results (you don't see previous images "behind" the
current image).

bjorn

-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Carl Eugen Hoyos]

2017-11-27 18:50 GMT+01:00 Bjorn Roche :
> Sorry for the very delayed response, Carl:
>
> On Tue, Nov 21, 2017 at 6:55 PM, Carl Eugen Hoyos 
> wrote:
>
>> 2017-11-21 18:53 GMT+01:00 Bjorn Roche :
>> > Support for transparencies in animated gifs requires modifying both
>> > libavcodec/gif.c and libavformat/gif.c because both the graphics
>> > control extension (handled by libavformat/gif.c) and the raw frame data
>> > (handled by libavcodec/gif.c) must be changed.
>>
>> Does remuxing animated gif with transparency work with your patch?
>
> A command like this:
>
> ffmpeg -i in.gif out.gif
>
> seems to produce the same output as before: transparency replaces
> with opacity.

remuxing == -vcodec copy

What about ffmpeg -i in.gif -vcodec copy out.gif ?

> However a command like this:
>
> ffmpeg -i in.gif  -vf palettegen -y /tmp/pal.png && ffmpeg -i in.gif
> -i /tmp/pal.png -lavfi paletteuse -y -f gif out.gif
>
> produces an output that looks like the input (including transparency).
>
> I believe this is a problem with the decoder, however, since
>
> ffmpeg -i /Users/bjorn/Desktop/smoketrail.gif
> /Users/bjorn/Desktop/smoketrail-out%d.png

This produces transparent images here for the output
file of ticket #6813.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Carl Eugen Hoyos]

2017-11-27 19:30 GMT+01:00 Bjorn Roche :
>
> On Fri, Nov 3, 2017 at 8:11 PM, Carl Eugen Hoyos  wrote:
>
>> 2017-10-24 18:40 GMT+02:00 Bjorn Roche :
>>
>> > - I don’t know if/how to update the FATE tests.
>>
>> A quick way (that needs some double-checking) is:
>> $ make GEN=1 SAMPLES=fate-suite fate
>>
>> (This overwrites all values with the new output.)
>
> Thanks. I ended up doing it manually to be sure, but this is useful!
>
>> Is the new side-data unavoidable?
>> (Would the only alternative be to merge the muxer into the
>> encoder?)
>
> The only way I can see is to move some functionality between the mixer and
> the encoder. It's not a clear line between the codec and the container in
> GIF, so I wouldn't object to an approach different from the one I took.

(Just a question!)

Does remuxing (animated) gif to gif work now? (I believe it does.)
Does it work with your patch?

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Bjorn Roche]

Thanks Moritz and Carl for your help with fate. This email was stuck in my
drafts :(

Comments below:

On Fri, Nov 3, 2017 at 8:11 PM, Carl Eugen Hoyos  wrote:

> 2017-10-24 18:40 GMT+02:00 Bjorn Roche :
>
> > - I don’t know if/how to update the FATE tests.
>
> A quick way (that needs some double-checking) is:
> $ make GEN=1 SAMPLES=fate-suite fate
>
> (This overwrites all values with the new output.)
>

Thanks. I ended up doing it manually to be sure, but this is useful!


> Is the new side-data unavoidable?
> (Would the only alternative be to merge the muxer into the
> encoder?)
>

The only way I can see is to move some functionality between the mixer and
the encoder. It's not a clear line between the codec and the container in
GIF, so I wouldn't object to an approach different from the one I took.

bjorn


-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Bjorn Roche]

Sorry for the very delayed response, Carl:

On Tue, Nov 21, 2017 at 6:55 PM, Carl Eugen Hoyos 
wrote:

> 2017-11-21 18:53 GMT+01:00 Bjorn Roche :
> > Support for transparencies in animated gifs requires modifying both
> > libavcodec/gif.c and libavformat/gif.c because both the graphics
> > control extension (handled by libavformat/gif.c) and the raw frame data
> > (handled by libavcodec/gif.c) must be changed.
>
> Does remuxing animated gif with transparency work with your patch?
>

A command like this:

ffmpeg -i in.gif out.gif

seems to produce the same output as before: transparency replaces with
opacity. However a command like this:

ffmpeg -i in.gif  -vf palettegen -y /tmp/pal.png && ffmpeg -i in.gif
-i /tmp/pal.png -lavfi paletteuse -y -f gif out.gif


produces an output that looks like the input (including transparency).


I believe this is a problem with the decoder, however, since


ffmpeg -i /Users/bjorn/Desktop/smoketrail.gif
/Users/bjorn/Desktop/smoketrail-out%d.png


produces opaque images as well.

-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs

[Carl Eugen Hoyos]

2017-11-21 18:53 GMT+01:00 Bjorn Roche :
> Support for transparencies in animated gifs requires modifying both
> libavcodec/gif.c and libavformat/gif.c because both the graphics
> control extension (handled by libavformat/gif.c) and the raw frame data
> (handled by libavcodec/gif.c) must be changed.

Does remuxing animated gif with transparency work with your patch?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Carl Eugen Hoyos]

2017-10-24 18:40 GMT+02:00 Bjorn Roche :

> - I don’t know if/how to update the FATE tests.

A quick way (that needs some double-checking) is:
$ make GEN=1 SAMPLES=fate-suite fate

(This overwrites all values with the new output.)

Is the new side-data unavoidable?
(Would the only alternative be to merge the muxer into the
encoder?)

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Moritz Barsnick]

On Thu, Nov 02, 2017 at 11:45:20 -0400, Bjorn Roche wrote:
> > - I don’t know if/how to update the FATE tests.
> 
> Can anyone comment on this? Do I update the tests in the same patch or
> separate that?

Your patch is not allowed to break fate. If the fate results are
correct references, then fate is catching an error in your patch which
you need to correct. If the change in fate is expected (implied by the
patch), then you need to change the fate results or adapt the fate test
within the same patch.

New additional fate tests are welcome (or preferred? not sure) as
separate patches.

How to update fate:
There's a web (or wiki) page about that, and James wrote this excellent
stuff recently (perhaps not related to your quest here):
http://ffmpeg.org/pipermail/ffmpeg-devel/2017-October/217473.html

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Bjorn Roche]

> - I don’t know if/how to update the FATE tests.


Can anyone comment on this? Do I update the tests in the same patch or
separate that?

-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Michael Niedermayer]

On Wed, Oct 25, 2017 at 09:51:39AM -0400, Bjorn Roche wrote:
> On Tue, Oct 24, 2017 at 8:56 PM, Michael Niedermayer  > wrote:
> 
> > On Tue, Oct 24, 2017 at 12:40:22PM -0400, Bjorn Roche wrote:
> > > Support for transparencies in animated gifs requires modifying both
> > > libavcodec/gif.c and libavformat/gif.c because both the graphics
> > > control extension (handled by libavformat/gif.c) and the raw frame data
> > > (handled by libavcodec/gif.c) must be changed. This is because
> > > transparencies in GIF can be used both to create a transparent image,
> > > and to provide optimization.
> > >
> > > How transparencies are interpreted in a given frame is controlled by
> > > the “disposal method”, which must be set appropriately in the graphics
> > > control extension.
> > >
> > >  The “in place” disposal method is used when transparency indicates
> > > optimization, and the “background” disposal method is used when
> > > transparency is intended to be preserved. In order to support both
> > > disposal methods, libavcodec/gif.c must signal to libavformat/gif.c
> > > which disposal method is required for every frame. This is done with a
> > > new side data type: AV_PKT_DATA_GIF_FRAME_DISPOSAL. This requires a
> > > change to avcodec.h
> > >
> > > Unfortunately, the addition of a new side data type causes some of the
> > > FATE tests to fail. This is not addressed here.
> > >
> > > This patch assumes paletteuse has already been patched to support
> > > transparency. (e.g. lavfi/paletteuse: fix to support transparency)
> > >
> > > Feedback I definitely need:
> > > - I’ve done a fair bit of testing, but I’m sure I missed some important
> > > cases.
> > > - I don’t know if/how to update the FATE tests.
> > > ---
> > >  libavcodec/avcodec.h |   6 ++
> > >  libavcodec/gif.c | 196 ++
> > +++--
> > >  libavformat/gif.c|  16 -
> > >  3 files changed, 212 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > > index 52cc5b0ca0..82a5328ce1 100644
> > > --- a/libavcodec/avcodec.h
> > > +++ b/libavcodec/avcodec.h
> > > @@ -1599,6 +1599,12 @@ enum AVPacketSideDataType {
> > >   */
> > >  AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> > >
> > > +/**
> > > + * The disposal method that should be used with the frame. If
> > missing,
> > > + * the frame will not be disposed. This contains exactly one byte.
> > > + */
> > > +AV_PKT_DATA_GIF_FRAME_DISPOSAL,
> > > +
> > >  /**
> > >   * ATSC A53 Part 4 Closed Captions. This metadata should be
> > associated with
> > >   * a video stream. A53 CC bitstream is stored as uint8_t in
> > AVPacketSideData.data.
> >
> > you cannot add values in the middle of a public enum, that would
> > change the ABI
> >
> 
> Ah, thanks -- I thought that was internal only. Is it safe to add to the
> end?

additions should be immedeatly before AV_PKT_DATA_NB
if this is insufficiently documented (seems so) then a patch improving
the documentation is welcome too

thx

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Bjorn Roche]

On Tue, Oct 24, 2017 at 8:56 PM, Michael Niedermayer  wrote:

> On Tue, Oct 24, 2017 at 12:40:22PM -0400, Bjorn Roche wrote:
> > Support for transparencies in animated gifs requires modifying both
> > libavcodec/gif.c and libavformat/gif.c because both the graphics
> > control extension (handled by libavformat/gif.c) and the raw frame data
> > (handled by libavcodec/gif.c) must be changed. This is because
> > transparencies in GIF can be used both to create a transparent image,
> > and to provide optimization.
> >
> > How transparencies are interpreted in a given frame is controlled by
> > the “disposal method”, which must be set appropriately in the graphics
> > control extension.
> >
> >  The “in place” disposal method is used when transparency indicates
> > optimization, and the “background” disposal method is used when
> > transparency is intended to be preserved. In order to support both
> > disposal methods, libavcodec/gif.c must signal to libavformat/gif.c
> > which disposal method is required for every frame. This is done with a
> > new side data type: AV_PKT_DATA_GIF_FRAME_DISPOSAL. This requires a
> > change to avcodec.h
> >
> > Unfortunately, the addition of a new side data type causes some of the
> > FATE tests to fail. This is not addressed here.
> >
> > This patch assumes paletteuse has already been patched to support
> > transparency. (e.g. lavfi/paletteuse: fix to support transparency)
> >
> > Feedback I definitely need:
> > - I’ve done a fair bit of testing, but I’m sure I missed some important
> > cases.
> > - I don’t know if/how to update the FATE tests.
> > ---
> >  libavcodec/avcodec.h |   6 ++
> >  libavcodec/gif.c | 196 ++
> +++--
> >  libavformat/gif.c|  16 -
> >  3 files changed, 212 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 52cc5b0ca0..82a5328ce1 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -1599,6 +1599,12 @@ enum AVPacketSideDataType {
> >   */
> >  AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> >
> > +/**
> > + * The disposal method that should be used with the frame. If
> missing,
> > + * the frame will not be disposed. This contains exactly one byte.
> > + */
> > +AV_PKT_DATA_GIF_FRAME_DISPOSAL,
> > +
> >  /**
> >   * ATSC A53 Part 4 Closed Captions. This metadata should be
> associated with
> >   * a video stream. A53 CC bitstream is stored as uint8_t in
> AVPacketSideData.data.
>
> you cannot add values in the middle of a public enum, that would
> change the ABI
>

Ah, thanks -- I thought that was internal only. Is it safe to add to the
end?

bjorn


-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Michael Niedermayer]

On Tue, Oct 24, 2017 at 12:40:22PM -0400, Bjorn Roche wrote:
> Support for transparencies in animated gifs requires modifying both
> libavcodec/gif.c and libavformat/gif.c because both the graphics
> control extension (handled by libavformat/gif.c) and the raw frame data
> (handled by libavcodec/gif.c) must be changed. This is because
> transparencies in GIF can be used both to create a transparent image,
> and to provide optimization.
> 
> How transparencies are interpreted in a given frame is controlled by
> the “disposal method”, which must be set appropriately in the graphics
> control extension.
> 
>  The “in place” disposal method is used when transparency indicates
> optimization, and the “background” disposal method is used when
> transparency is intended to be preserved. In order to support both
> disposal methods, libavcodec/gif.c must signal to libavformat/gif.c
> which disposal method is required for every frame. This is done with a
> new side data type: AV_PKT_DATA_GIF_FRAME_DISPOSAL. This requires a
> change to avcodec.h
> 
> Unfortunately, the addition of a new side data type causes some of the
> FATE tests to fail. This is not addressed here.
> 
> This patch assumes paletteuse has already been patched to support
> transparency. (e.g. lavfi/paletteuse: fix to support transparency)
> 
> Feedback I definitely need:
> - I’ve done a fair bit of testing, but I’m sure I missed some important
> cases.
> - I don’t know if/how to update the FATE tests.
> ---
>  libavcodec/avcodec.h |   6 ++
>  libavcodec/gif.c | 196 
> +--
>  libavformat/gif.c|  16 -
>  3 files changed, 212 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 52cc5b0ca0..82a5328ce1 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -1599,6 +1599,12 @@ enum AVPacketSideDataType {
>   */
>  AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
>  
> +/**
> + * The disposal method that should be used with the frame. If missing,
> + * the frame will not be disposed. This contains exactly one byte.
> + */
> +AV_PKT_DATA_GIF_FRAME_DISPOSAL,
> +
>  /**
>   * ATSC A53 Part 4 Closed Captions. This metadata should be associated 
> with
>   * a video stream. A53 CC bitstream is stored as uint8_t in 
> AVPacketSideData.data.

you cannot add values in the middle of a public enum, that would
change the ABI

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

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Bjorn Roche]

On Tue, Oct 24, 2017 at 4:24 PM, Carl Eugen Hoyos 
wrote:

> 2017-10-24 18:40 GMT+02:00 Bjorn Roche :
>
> > This patch assumes paletteuse has already been patched to support
> > transparency. (e.g. lavfi/paletteuse: fix to support transparency)
>
> Which should - imo - be unrelated to this patch and indeed, transcoding
> a pal8 apng (produced with your paletteuse patch) produces a good gif
> with this patch.
> I don't know if this is the best approach though.
>

Ah, okay, then my comment is wrong -- I just didn't see a way to test it
without paletteuse being fixed, but a pal8 apng makes sense.

bjorn

-- 


Bjorn Roche

Sr. Video Pipeline Engineer

bj...@giphy.com
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Carl Eugen Hoyos]

2017-10-24 18:40 GMT+02:00 Bjorn Roche :

[...]

The patch adds two warnings here on compilation of gif.c,
they should be fixed at some point:

libavcodec/gif.c:164:5: warning: ISO C90 forbids mixed declarations
and code [-Wdeclaration-after-statement]
 uint8_t *frame_disposal = av_packet_new_side_data(pkt,
AV_PKT_DATA_GIF_FRAME_DISPOSAL, 1);
 ^~~
libavcodec/gif.c: In function ‘gif_image_write_translucent’:
libavcodec/gif.c:333:5: warning: ISO C90 forbids mixed declarations
and code [-Wdeclaration-after-statement]
 uint8_t *frame_disposal = av_packet_new_side_data(pkt,
AV_PKT_DATA_GIF_FRAME_DISPOSAL, 1);
 ^~~

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Carl Eugen Hoyos]

2017-10-24 18:40 GMT+02:00 Bjorn Roche :

> This patch assumes paletteuse has already been patched to support
> transparency. (e.g. lavfi/paletteuse: fix to support transparency)

Which should - imo - be unrelated to this patch and indeed, transcoding
a pal8 apng (produced with your paletteuse patch) produces a good gif
with this patch.
I don't know if this is the best approach though.

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] fix for transparencies in animated gifs (requires feedback)

[Bjorn Roche]

Support for transparencies in animated gifs requires modifying both
libavcodec/gif.c and libavformat/gif.c because both the graphics
control extension (handled by libavformat/gif.c) and the raw frame data
(handled by libavcodec/gif.c) must be changed. This is because
transparencies in GIF can be used both to create a transparent image,
and to provide optimization.

How transparencies are interpreted in a given frame is controlled by
the “disposal method”, which must be set appropriately in the graphics
control extension.

 The “in place” disposal method is used when transparency indicates
optimization, and the “background” disposal method is used when
transparency is intended to be preserved. In order to support both
disposal methods, libavcodec/gif.c must signal to libavformat/gif.c
which disposal method is required for every frame. This is done with a
new side data type: AV_PKT_DATA_GIF_FRAME_DISPOSAL. This requires a
change to avcodec.h

Unfortunately, the addition of a new side data type causes some of the
FATE tests to fail. This is not addressed here.

This patch assumes paletteuse has already been patched to support
transparency. (e.g. lavfi/paletteuse: fix to support transparency)

Feedback I definitely need:
- I’ve done a fair bit of testing, but I’m sure I missed some important
cases.
- I don’t know if/how to update the FATE tests.
---
 libavcodec/avcodec.h |   6 ++
 libavcodec/gif.c | 196 +--
 libavformat/gif.c|  16 -
 3 files changed, 212 insertions(+), 6 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 52cc5b0ca0..82a5328ce1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1599,6 +1599,12 @@ enum AVPacketSideDataType {
  */
 AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
 
+/**
+ * The disposal method that should be used with the frame. If missing,
+ * the frame will not be disposed. This contains exactly one byte.
+ */
+AV_PKT_DATA_GIF_FRAME_DISPOSAL,
+
 /**
  * ATSC A53 Part 4 Closed Captions. This metadata should be associated with
  * a video stream. A53 CC bitstream is stored as uint8_t in 
AVPacketSideData.data.
diff --git a/libavcodec/gif.c b/libavcodec/gif.c
index d9c99d52cf..6c839d99d2 100644
--- a/libavcodec/gif.c
+++ b/libavcodec/gif.c
@@ -74,11 +74,35 @@ static int pick_palette_entry(const uint8_t *buf, int 
linesize, int w, int h)
 return -1;
 }
 
-static int gif_image_write_image(AVCodecContext *avctx,
- uint8_t **bytestream, uint8_t *end,
- const uint32_t *palette,
- const uint8_t *buf, const int linesize,
- AVPacket *pkt)
+// returns true if any of the pixels are transparent
+static int is_image_translucent(AVCodecContext *avctx,
+const uint32_t *palette,
+const uint8_t *buf, const int linesize)
+{
+GIFContext *s = avctx->priv_data;
+int trans = s->transparent_index;
+int p;
+const int m = avctx->width * avctx->height ;
+
+if (trans < 0) {
+return 0;
+}
+
+for (p=0; ppriv_data;
 int len = 0, height = avctx->height, width = avctx->width, x, y;
@@ -137,6 +161,11 @@ static int gif_image_write_image(AVCodecContext *avctx,
width, height, x_start, y_start, avctx->width, avctx->height);
 }
 
+uint8_t *frame_disposal = av_packet_new_side_data(pkt, 
AV_PKT_DATA_GIF_FRAME_DISPOSAL, 1);
+if (!frame_disposal)
+return AVERROR(ENOMEM);
+*frame_disposal = GCE_DISPOSAL_INPLACE;
+
 /* image block */
 bytestream_put_byte(bytestream, GIF_IMAGE_SEPARATOR);
 bytestream_put_le16(bytestream, x_start);
@@ -214,6 +243,163 @@ static int gif_image_write_image(AVCodecContext *avctx,
 return 0;
 }
 
+// wrtites an image that may contain transparency
+// this should work for opaque images as well, but will be less optimized.
+static int gif_image_write_translucent(AVCodecContext *avctx,
+   uint8_t **bytestream, uint8_t *end,
+   const uint32_t *palette,
+   const uint8_t *buf, const int linesize,
+   AVPacket *pkt)
+{
+GIFContext *s = avctx->priv_data;
+int len = 0, height =