[FFmpeg-devel] [PATCH] avcodec[/format]/webpenc: use WebPAnimEncoder API to generate animated WebP

[Urvang Joshi]

On Thu, Apr 23, 2015 at 2:51 PM Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Thu, Apr 16, 2015 at 10:40:14PM +0000, Urvang Joshi wrote:
> > On Thu, Apr 16, 2015 at 3:09 PM James Almer <jamrial at gmail.com> wrote:
> >
> > > On 16/04/15 4:18 PM, Urvang Joshi wrote:
> > > > Hi,
> > > > Here's the patch without whitespace changes.
> > > >
> > > > Thanks,
> > > > Urvang
> > >
> > > This patch doesn't apply cleanly. Looks like something weird with the
> > > indentation still.
> > > Was this patch handmade? It says the hash for libwebpenc.c is 95d56ac
> > > (same as git head),
> > > but the contents of the patch don't match.
> > >
> >
> > Sorry, I should have mentioned that it was created with
> > "--ignore-all-space" option, so using the same option when applying the
> > patch would have worked.
> >
> > But to avoid any confusion, here's the re-created patch, that should
> apply
> > cleanly with just 'git am'.
> >
> >
> > >
> > > After fixing the conflicts and compiling the patch seems to work, but
> the
> > > resulting
> > > animated webp files are smaller than those using the native muxer using
> > > the default
> > > encoding and muxing settings.
> > > Is this because the muxing done by libwebpmux is different, or are the
> > > quality defaults
> > > changed in any way when using this codepath? If the former then that's
> > > pretty good, but
> > > if the latter then it should probably be fixed.
> > >
> >
> > Short answer: muxing done by libwebpmux is different, so it's expected
> that
> > it generates smaller WebP files.
> >
> > Detailed answer:
> > The native muxer is naive, and it always uses X offset and Y offset of 0
> > for all frames. This means the full width x height of all frames are
> > encoded.
>
> > libwebpmux muxer is smart on the other hand: for example, it only encodes
> > the part of the frame which has changed from previous frame.
> > This and other optimizations result in smaller WebP files.
>
> can you show some PSNR vs filesize information ?
>

As explained below, we aren't changing the underlying encoder -- only the
muxer is being changed.


>
>
> >
> > Thanks,
> > Urvang
> >
> >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
>
> >  configure               |    5 ++
> >  libavcodec/libwebpenc.c |   93
> +++++++++++++++++++++++++++++++++++++++++++-----
> >  libavformat/webpenc.c   |   44 ++++++++++++++++++++++
>
> why are there changes to libavformat in a patch changing an encoder?
>

Note that WebPAnimEncoder API used now internally uses WebP encoder and
WebP muxer.
Earlier, ffmpeg was using WebP encoder with its native muxer.

So, we are only changing the muxer here, the underlying encoder is still
the WebPEncoder as earlier.


>
>
> [...]
> > index ee110de..0162eff 100644
> > --- a/libavformat/webpenc.c
> > +++ b/libavformat/webpenc.c
> > @@ -22,12 +22,25 @@
> >  #include "libavutil/intreadwrite.h"
> >  #include "libavutil/opt.h"
> >  #include "avformat.h"
> > +#include "config.h"
> >  #include "internal.h"
> >
> > +#if CONFIG_LIBWEBP
> > +#include <webp/encode.h>
> > +#if HAVE_WEBP_MUX_H
> > +#include <webp/mux.h>
> > +#if (WEBP_MUX_ABI_VERSION >= 0x0105)
> > +#define USE_WEBP_ANIMENCODER
> > +#endif
> > +#endif
> > +#endif
> > +
> >  typedef struct WebpContext{
> >      AVClass *class;
> >      int frame_count;
> > +#ifndef USE_WEBP_ANIMENCODER
> >      AVPacket last_pkt;
> > +#endif
> >      int loop;
> >  } WebpContext;
> >
> > @@ -44,13 +57,40 @@ static int webp_write_header(AVFormatContext *s)
> >          av_log(s, AV_LOG_ERROR, "Only WebP is supported\n");
> >          return AVERROR(EINVAL);
> >      }
> > -    avpriv_set_pts_info(st, 24, 1, 1000);
> >
> > +#ifndef USE_WEBP_ANIMENCODER
> > +    avpriv_set_pts_info(st, 24, 1, 1000);
> >      avio_write(s->pb, "RIFF\0\0\0\0WEBP", 12);
> > +#endif
> > +
> > +    return 0;
> > +}
>
> this removes the timebase setting, that looks wrong
>

Earlier, "AVCodec ff_libwebp_encoder" was NOT using "CODEC_CAP_DELAY"
capability, so "avpkt->pts" was computed automatically:
https://www.ffmpeg.org/doxygen/2.5/libavcodec_2utils_8c_source.html#l02123

And the same was used to compute the duration of the current frame:
http://ffmpeg.org/doxygen/trunk/webpenc_8c_source.html#l00103

But, the new code DOES use "CODEC_CAP_DELAY" capability, and computes the
timestamp of a frame explicitly as "avctx->time_base.num * frame->pts *
1000 / avctx->time_base.den;". So, IIUC, it doesn't need to call
avpriv_set_pts_info().

Am I missing anything?


>
> also please make sure -vcodec copy does if it works before the
> patch, still work afterwards
>

In short, the "-vcodec copy" behavior is same before/after the patch.

In detail:
(i) vcodec copy from video/GIF to WebP (e.g. "./ffmpeg -i input.mp4 -vcodec
copy output.webp") would NOT work before or after, due to the expected
error "Only WebP is supported":
http://ffmpeg.org/doxygen/trunk/webpenc_8c_source.html#l00044

(ii) vcodec copy from animated WebP to animated WebP (e.g. "./ffmpeg -i
animated.webp -vcodec copy output.webp") would NOT work before or after
either, due to ANMF chunk being unsupported in the native demuxer:
http://ffmpeg.org/doxygen/trunk/webp_8c_source.html#l01499

(iii) vcodec copy from static WebP to static WebP (e.g. "./ffmpeg -i
static.webp -vcodec copy output.webp") WOULD work both before and after.



> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Republics decline into democracies and democracies degenerate into
> despotisms. -- Aristotle
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>