[Theora-dev] Re: MMX/mmxext optimisations

[Wim Taymans]

On Wed, 2004-08-25 at 23:07, Ralph Giles wrote:
> On Wed, Aug 25, 2004 at 10:10:22PM +0100, Jim Darby wrote:
> 
> > > One down size is that to my eye, mmx-encoded theora videos are
> > > slightly blurrier than their standard counterparts, for a fixed
> > > target bitrate.  Also, for a fixed quality factor, the files seem
> > > quite a lot smaller (but not particularly worse-looking on brief
> > > inspection, which is odd and counter-intuitive given the first
> > > observation, but I'm not complaining).
> > 
> > This is a little worrying. Surely the MMX code should just make things
> > happen faster rather than different?
> 
> Indeed. Of course it's easy to fool yourself, but particularly because 
> errors are cumulative between I frames, the in-loop stuff does need
> to be per spec.
> 
> Wim, can you confirm you didn't include any short cuts?

No shortcuts are included. Most optimisations were verified by comparing
the C and mmx versions. No formal comparison framework or testing has
been done. I did not compare the fdct version at all, it was a verbatim
port of the VP3 assembler and I was satisfied when the results were
visually acceptable.

as Rodolpho said, the averaging code in the mmxext version does rounding
while the C version doesn't. That combined with (probably) a different
output from the fdct could explain the difference.

oh, right. GetHalfPixelSumAbsDiffs in the reference encoder has a bug
where ErrorSoFar is added twice to DiffVall. That would also make a
difference. Error like this:

--- mcomp.c     2003-12-03 09:59:41.000000000 +0100
+++ mcomp.c.fix 2004-08-26 15:40:48.000000000 +0200
@@ -272,7 +272,7 @@ static ogg_uint32_t GetHalfPixelSumAbsDi

   if ( RefOffset == 0 ) {
     /* Simple case as for non 0.5 pixel */
-    DiffVal += GetSumAbsDiffs( SrcData, RefDataPtr1, PixelsPerLine,
+    DiffVal = GetSumAbsDiffs( SrcData, RefDataPtr1, PixelsPerLine,
                                ErrorSoFar);
   } else  {
     for ( i=0; i < BLOCK_HEIGHT_WIDTH; i++ ) {


Regards,
Wim

> 
>  -r
> _______________________________________________
> Theora-dev mailing list
> Theora-dev at xiph.org
> http://lists.xiph.org/mailman/listinfo/theora-dev