[FFmpeg-devel] [PATCH 1/4] lavc/h263dsp: add DCT dequantisation functions

[Andreas Rheinhardt]

Rémi Denis-Courmont:
> Le lauantaina 6. heinäkuuta 2024, 19.19.35 EEST Andreas Rheinhardt a écrit :
>> Rémi Denis-Courmont:
>>> Le lauantaina 6. heinäkuuta 2024, 18.17.37 EEST Andreas Rheinhardt a écrit 
> :
>>>> Rémi Denis-Courmont:
>>>>> Note that optimised implementations of these functions will be taken
>>>>> into actual use only if MpegEncContext.dct_unquantize_h263_{inter,intra}
>>>>> are *not* overloaded by existing optimisations.
>>>>> ---
>>>>>
>>>>>  libavcodec/h263dsp.c | 25 +++++++++++++++++++++++++
>>>>>  libavcodec/h263dsp.h |  4 ++++
>>>>>  2 files changed, 29 insertions(+)
>>>>>
>>>>> diff --git a/libavcodec/h263dsp.c b/libavcodec/h263dsp.c
>>>>> index 6a13353499..dc146bf821 100644
>>>>> --- a/libavcodec/h263dsp.c
>>>>> +++ b/libavcodec/h263dsp.c
>>>>> @@ -19,10 +19,33 @@
>>>>>
>>>>>  #include <stdint.h>
>>>>>  
>>>>>  #include "libavutil/attributes.h"
>>>>>
>>>>> +#include "libavutil/avassert.h"
>>>>>
>>>>>  #include "libavutil/common.h"
>>>>>  #include "config.h"
>>>>>  #include "h263dsp.h"
>>>>>
>>>>> +static void h263_dct_unquantize_inter_c(int16_t *block, ptrdiff_t len,
>>>>> +                                        int qmul, int qadd)
>>>>> +{
>>>>> +    for (ptrdiff_t i = 0; i <= len; i++) {
>>>>
>>>> In the current code, the unquantizing inter does not need an initial
>>>> check (the compiler can optimize the for-loop to a do-while-like loop).
>>>> This will be lost here.
>>>
>>> If this not the consequence of your own actions...
>>>
>>> You asked:
>>> - to use a signed type there, and
>>> - to avoid the separate initial branch in the intra case.
>>>
>>> And that is exactly how this ends up. You can't have it both ways.
>>
>> One can have it both ways: Use a do-while loop.
> 
> That is only possible if the otherwise identical intra/inter code are 
> duplicated, or if putting back the initial check in the intra case. The very 
> check which you asked to remove.
> 
> So no, you can't have it both ways. I don't really care either way, but the 
> check for intra needs to be *somewhere*.
> 

No, it need not exist. The point of raster_end etc. is purely to
shortcut this loop. It would be correct to always use 63 for nCoeffs. In
fact, it seems to me that dct_unquantize_h263_inter_mmx() already uses a
do-while-like loop.

- Andreas