[FFmpeg-devel] ffvorbis, inverted output

[Michael Niedermayer]

On Fri, Oct 03, 2008 at 02:50:50AM +0300, Siarhei Siamashka wrote:
> On Monday 29 September 2008, Michael Niedermayer wrote:
> > On Mon, Sep 29, 2008 at 09:42:35AM +0300, Siarhei Siamashka wrote:
> > > On Sunday 28 September 2008, Michael Niedermayer wrote:
> > > > On Sat, Sep 27, 2008 at 09:59:21PM +0300, 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
> > > >
> > > > [...]
> > > >
> > > > > @@ -98,6 +108,11 @@
> > > > >      return -1;
> > > > >  }
> > > > >
> > > > > +int ff_mdct_init(MDCTContext *s, int nbits, int inverse)
> > > > > +{
> > > > > +    return ff_mdct_init_scaled(s, nbits, inverse, 1.0);
> > > > > +}
> > > > > +
> > > > >  /* complex multiplication: p = a * b */
> > > > >  #define CMUL(pre, pim, are, aim, bre, bim) \
> > > > >  {\
> > > >
> > > > i think its better to just add scale to ff_mdct_init()
> > >
> > > OK, should I resubmit a patch to also include changes for all
> > > the other occurrences of 'ff_mdct_init' to use scale factor 1.0?
> >
> > yes, please
> 
> Updated patch series attached.

looks ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081004/6a2c6be8/attachment.pgp>