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

Urvang Joshi urvang at google.com
Thu May 7 23:28:43 CEST 2015


On Thu, Apr 30, 2015 at 12:05 PM Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Wed, Apr 29, 2015 at 11:53:40PM +0000, Urvang Joshi wrote:
> > On Mon, Apr 27, 2015 at 5:03 PM Michael Niedermayer <michaelni at gmx.at>
> > wrote:
> >
> > > On Mon, Apr 27, 2015 at 10:18:52PM +0000, Urvang Joshi wrote:
> > > > 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.
> > >
> > > Ok, so the patch adds many #ifs to both the muxer and
> > > encoder, and there are more changes in the encoder than the muxer
> > > the commit message which is 1 single line only speaks about the encoder
> > > and the patch is only about the muxer.
> > > Did i understand it correctly this time ?
> > >
> > > assuming iam not entirely wrong here.
> > > First question is what does the patch actually try to achive ?
> > > replace a native muxer by a new external dependancy ?
> > > if so, why would we want that ?
> > >
> >
> > In theory, here are some of the optimizations that AnimEncoder API (if
> > available) does to be more efficient than native muxer:
> > - Pick the best dispose/blend method combination for each frame.
> > - Based on the dispose/blend method, encode only a sub-rectangle of the
> > frame that has changed from the previous (disposed) frame.
> > - If some pixels in current frame match the corresponding pixels in the
> > previous frame, possibly turn them into transparent pixels (so that they
> > see through the previous frame's pixel), if that improves compression.
> > - Optionally, it can also insert keyframes at regular intervals (not used
> > in this patch; but I plan to add a command-line option to allow this in
> > future).
>
> lets try to agree on terminology first:
> Code which compresses that rectangular area of pixels called
> a picture or frame into a compressed bitstream is called an encoder
> deciding how to encode a frame, frame area or pixel is all encoder
> side.
> In that light comparing "the native muxer" against AnimEncoder API
> which does all kinds of smart encoder steps is very odd thats like
> comparing debian against the linux kernel aka it makes no sense at all
>

Sorry if this wasn't clear.
To be precise, I was comparing "WebPEncode + Native muxer" to "AnimEncoder"
(which is nothing but WebPEncode + libwebpmux).


>
> 2nd our encoder supports reusing pixels from the previous frame.
> AnimEncoder might be better at it or it might be worse but its not a
> feature that isnt already supported
> the feature is tuned by the cr_threshold / cr_size options as i
> said previously.
>
> now if AnimEncoder is better than what we have ATM, i _do_ want it
> supported but do NOT add this code with #ifdefs in the existing
> encoder.
> create a new file, and move the code into it.


I can investigate how feasible the splitting is. My concerns with splitting
into two files are:
- There is some common code which will be duplicated in the two files.
- How would one muxer be picked over the other (with they being in separate
files)? Ideally, it would good if the libwebpmux muxer is automatically
picked when available, instead of the user explicitly having to pick the
same.


> we also dont have the
> xvid encoder in our mpeg4 encoder with #ifdefs
>
>
>
>
> >
> > In practice, here is some data to compare the new AnimEncoder with
> ffmpeg's
> > native muxer --
> >
> > I took ~7000 animated GIFs off the web and converted them to animated
> WebP
> > using the native muxer as well as the libwebpmux (both in "-lossless 1"
> and
> > "-lossless 0" modes). Here are the total output filesizes:
> >
> > Lossless:
> > Native muxer: 2187508560 bytes
> > AnimEncoder: 1449909662 bytes
> > (33.7% saving)
> >
> > Lossy:
> > Native muxer: 872878478 bytes
> > AnimEncoder: 645406034 bytes
> > (26% saving)
>
> The standard procedure is to use raw RGB or raw YUV input
> material
>

Animated image use case is a bit special compared to videos in general --
some animated images are often graphical with many parts of the frame not
changing between successive frames.

That is why I chose a set of GIF images from the web -- they cover the two
typical use cases: graphical animations and short video clips.


> and for comparing lossy compression its needed to draw a quality vs
> bitrate graph.
> that is one lossy compressor cant be compared to another just by
> filesize, the quality matters too
>
>
Indeed, we need to make sure that the new muxer + encoder combo has smaller
size at the same quality.

For this, I used the anim_diff test tool (from
*https://chromium.googlesource.com/webm/libwebp/+/master/examples/anim_diff.cc
<https://chromium.googlesource.com/webm/libwebp/+/master/examples/anim_diff.cc>*)
and modified it a bit to get average per-frame PSNR numbers for the ~7000
images tested earlier.

I ran the same at different "-quality" values (0, 25, 50, 75, 100). The
Average PSNR vs total-filesize curves for the current and new code attached.

Thanks,
Urvang


>
> >
> > So, there is a clear benefit of using AnimEncoder when it's available.
> >
> >
> > >
> > > And assuming we do want that (which iam not sure we do)
> > > why are there changes in the encoder ?
> > >
> >
> > AnimEncoder API used in the patch is a combination of encoder
> (WebPEncode)
> > and muxer (WebPMux).
> >
> > So, when this new API is used:
> > - In the encoder layer: we use WebPAnimEncoderAdd() instead of
> WebPEncode().
> > - The muxer layer: has to work like a raw muxer.
> >
> > On the other hand, when new API isn't available, the old code is used as
> it
> > is:
> > - In the codec layer: WebPEncode is used to encode each frame
> > - In the muxer layer:  ffmpeg muxer is used
>
> like i said above, please put the new codec layer stuff in a new file
> dont intermingle it into an existing one with ifdefs
>
>
> >
> >
> > > and independant of above, changes to codec and muxer layer should be
> > > seperate patches
> >
> >
> > As explained above, the changes to the codec and muxer layers are
> dependent
> > in this case; so they cannot be split.
>
> iam sure the muxer can read the chunk type to determine based on
> that if its feeded with a RIFF webp file or the bitstream of frames
> from inside a webp.
> And when it can do so it can switch between the 2 muxing modes
> so the changes not only can be split but its even trivial to do so
>
> of course this still introduces a different format for the new
> encoders AVPackets, i dont know if thats a good idea but its
> something the community has to decide thats not for me to do
> but if noone complains then i guess noone minds
>
> Thanks
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Avoid a single point of failure, be that a person or equipment.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: output.png
Type: image/png
Size: 11168 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150507/c588fac2/attachment.png>


More information about the ffmpeg-devel mailing list