[Theora-dev] Re: MMX/mmxext optimisations

[Adam D. Moss]

Without going back and looking at the code (I SO LAZY!) the change
to the patch does mildly worry me because it looks like a semantic
change: now the ErrorSoFar variable is not being factored into the
resulting sum at all.  If ErrorSoFar==DiffVal (before the call) then
the two patches are equivilent, otherwise...
(Though even if they are different, I have no idea if your version
is actually the less correct of the two.)
--Adam

Ralph Giles wrote:
> Thanks for finding this. I committed a different patch, just passing 
> '0' for ErrorSoFar on recursion. Seemed wrong to break the accumulation 
> convention.
> 
>  -r
> 
> On Fri, Aug 27, 2004 at 09:06:02AM +0200, Wim Taymans wrote:
> 
> 
>>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