[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 <michael at niedermayer.cc
> > 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171025/4cadc3d9/attachment.sig>