On Thu, May 07, 2015 at 09:28:43PM +0000, Urvang Joshi wrote: > On Thu, Apr 30, 2015 at 12:05 PM Michael Niedermayer <michae...@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 <michae...@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 < > > michae...@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 <jamr...@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@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 > - 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
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel