[FFmpeg-devel] [PATCH] h264 deblocking result doesn'tmatch with SIMD on and off.

[David Conrad]

On May 23, 2009, at 9:03 PM, Michael Niedermayer wrote:

> On Sat, May 23, 2009 at 08:56:49PM -0400, Jason Garrett-Glaser wrote:
>> On Sat, May 23, 2009 at 8:35 PM, Haruhiko Yamagata <h.yamagata at nifty.com 
>> > wrote:
>>>> On Sun, May 24, 2009 at 12:40:18AM +0900, Haruhiko Yamagata wrote:
>>>>>
>>>>> In h264 decoder, deblocking result doesn't match with SIMD on  
>>>>> and off.
>>>>>
>>>>> In h264_loop_filter* (dsputil.c), there are several
>>>>>
>>>>>   if( FFABS( p0 - q0 ) < alpha &&
>>>>>       FFABS( p1 - p0 ) < beta &&
>>>>>       FFABS( q1 - q0 ) < beta )
>>>>>
>>>>> In SIMD optimized functions, this is done in H264_DEBLOCK_MASK
>>>>> (h264dsp_mmx.c).
>>>>>
>>>>>   if (!(FFABS( p0 - q0 ) > (uint8_t)(alpha -1) ||
>>>>>         FFABS( p1 - p0 ) > (uint8_t)(beta  -1) ||   // beta ==  
>>>>> 0 ? Oops :
>>>>> ...
>>>>>         FFABS( q1 - q0 ) > (uint8_t)(beta  -1) ))
>>>>>
>>>>> So the SIMD optimized functions expect alpha and beta not to be  
>>>>> zero.
>>>>> OK, let them assume it for performance.
>>>>> The attached patch fixes this issue.
>>>>>
>>>>> The errors are usually invisible, but after a long chain of  
>>>>> reference,
>>>>> the errors accumulate and eventually get visible.
>>>>>
>>>>> Best regards,
>>>>> Haruhiko Yamagata
>>>>> ffdshow-tryouts
>>>>>
>>>>
>>>>>  h264.c |    4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>> 089aea8ce540d5087b27de54b2a67b7b11e8d707   
>>>>> dont_deblock_if_beta_is_0.patch
>>>>
>>>> This patch adds the checks so they are also done for the C code
>>>> this seems wastefull unless there is some advantage in it?
>>>
>>> If the stream has alpha ==0 or beta==0, the performance is better  
>>> with the
>>> patch. But it doesn't seem to be common enough.
>>
>> That'll be the case in any macroblock with quantizer 15 or below.
>>
>> If you're using, say, --deblock -2:-2, that would be 19 or below.
>> With x264's AQ, that'll occur all the time, so I'd say keep it in  
>> even
>> with C.  We have that line in x264 as well.
>
> well then iam ok with it too

Applied