[FFmpeg-devel] ffvorbis, inverted output

[Siarhei Siamashka]

On Sunday 28 September 2008, Benjamin Larsson wrote:
> Siarhei Siamashka wrote:
> > On Saturday 27 September 2008, Siarhei Siamashka wrote:
> >> On Sunday 14 September 2008, Michael Niedermayer wrote:
> >>> On Sun, Sep 14, 2008 at 04:50:59AM +0300, Siarhei Siamashka wrote:
> >>>> Hi,
> >>>>
> >>>> I tried to do PSNR comparison of libvorbis/ffvorbis/tremor and noticed
> >>>> that output from ffvorbis is actually inverted (ex. output 0xFFFF in
> >>>> libvorbis corresponds to 0x0001 in ffvorbis and so on) when compared
> >>>> to the output from the other decoders.
> >>>>
> >>>> Should this be fixed?
> >>>
> >>> yes, if all (/most) other vorbis decoders match and we differ from that
> >>
> >> Can these two patches be used as a fix? The first one adds support for
> >> scaled imdct output. The second one inverts output of the decoder (to
> >> match libvorbis and tremor) using negative scale factor.
> >>
> >> As additional bonus, 'copy_normalize' function from vorbis decoder is
> >> simplified. Though I get some inconsistent benchmark results
> >> (performance difference is negligible with one or another variant
> >> getting ahead randomly) and would like someone to confirm that there is
> >> no performance regression.
> >>
> >> Getting scaled imdct output involves sqrt operation and scale factor
> >> uses odd power of two, so there is some difference in PSNR compared to
> >> SVN trunk (taking inversion into account):
> >>
> >> stddev:    0.02 PSNR:127.97 bytes: 22057216/ 22057216
> >
> > Oops, an extra patch to ensure tables alignment is also needed. It is
> > attached.
> >
> > Regarding performance, it really provides a visible improvement on some
> > files (the ones which have lots of block size switching and consequently
> > lots of 'copy_normalize' calls). After having done more tests, I observed
> > vorbis decoding speedup up to 1%. In any case, output scaling support in
> > IMDCT with zero overhead should be a good thing to have and might be
> > useful in other decoders too.
>
> I posted a patch a long time ago that calculated the window on the fly.
> With the code it would be easy to scale the window in any way you want
> and it would be as fast as now.

Cool. I can't find it now in the mailing list archives (link is always
welcome), but do you know why it was not applied yet?

In any case, just having a scaled window is not enough (unless I'm
misunderstanding you). On the boundary of different sized blocks we need
to process a 'flat' part of a window which is performed by that 
'copy_normalize' function. If the scaling was done on window only, the
function 'copy_normalize' would become slower and not turn into a simple
memcpy operation.

I also think that interaction between 'ff_imdct_half' and 'vector_fmul_window'
is not very optimal now and involves many redundant load/store operations and
memcpy calls while decoding. Having imdct extended to support not just a
constant scale factor in output, but arbitrary shape with little or no
overhead would probably help to improve performance a lot. I will not feel
satisfied until I try everything that I can to push ffvorbis decoder
performance up to its limits :)

But even a constant scale factor is useful, I also have some other 
interest in it: http://article.gmane.org/gmane.comp.video.ffmpeg.devel/74941/

-- 
Best regards,
Siarhei Siamashka