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

Urvang Joshi urvang at google.com
Fri May 15 03:29:19 CEST 2015 

On Fri, May 8, 2015 at 8:22 AM Michael Niedermayer <michaelni at gmx.at> wrote:

> On Thu, May 07, 2015 at 09:28:43PM +0000, Urvang Joshi wrote:
> > 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.
>
> common parts could be put in a common file and/or shared
>

Just split the two encoders as well as muxers in separate files (with
common code in "_common.[hc]" files).

Please take another look.


>
>
> > - 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.
>
> the order in which they are registered could cause one to be favored
> if nothing explicit is requested
>
>
> >
> >
> > > 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
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I know you won't believe me, but the highest form of Human Excellence is
> to question oneself and others. -- Socrates
> _______________________________________________
> 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: split_files.ffmpeg_animated_webp.patch
Type: text/x-patch
Size: 48270 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150515/62b9a9db/attachment.bin>

More information about the ffmpeg-devel mailing list