[FFmpeg-devel] [PATCH]levc/hevc_cabac Optimise ff_hevc_hls_residual_coding (especially ARM)

[James Almer]

On 1/19/2016 2:24 PM, John Cox wrote:
> On Tue, 19 Jan 2016 14:09:22 -0300, you wrote:
> 
>> On 1/19/2016 2:05 PM, John Cox wrote:
>>>> On 1/19/2016 9:46 AM, John Cox wrote:
>>>>> +// Helper fns
>>>>> +#ifndef hevc_mem_bits32
>>>>> +static av_always_inline uint32_t hevc_mem_bits32(const void * buf, const unsigned int offset)
>>>>> +{
>>>>> +    return AV_RB32((const uint8_t *)buf + (offset >> 3)) << (offset & 7);
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> +#if AV_GCC_VERSION_AT_LEAST(3,4) && !defined(hevc_clz32)
>>>>> +#define hevc_clz32 hevc_clz32_builtin
>>>>> +static av_always_inline unsigned int hevc_clz32_builtin(const uint32_t x)
>>>>> +{
>>>>> +    // __builtin_clz says it works on ints - so adjust if int is >32 bits long
>>>>> +    return __builtin_clz(x) - (sizeof(int) * 8 - 32);
>>>>
>>>> Why aren't you simply using ff_clz?
>>>
>>> Because it doesn't exist? or at least I can't find it.
>>>
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> +// It is unlikely that we will ever need this but include for completeness
>>>>
>>>> There are at least two compilers we support that don't define __GNUC__, so
>>>> it would be used.
>>>> And in any case, isn't all this duplicating ff_clz, which is available in
>>>> libavutil/inthmath.h?
>>>
>>> Are you sure of that?  I can find ff_ctz but no ff_clz...
>>> I would happily be wrong.
>>
>> I assume you're writing this patch for the ffmpeg 2.8 branch or older, which you shouldn't.
>> Always use the master branch. You'll find ff_clz there.
> 
> Yes/no - the code I wrote had to work against 2.8 as that is what
> Rasperry Pi are using at the moment.  This patch is meant to be against
> master so I can/will happily remove that code. (And I had the wrong
> version checked out when commenting previously)
> 
> By the way - can you tell me what the behaviour of ff_clz is when ints
> are 64 bits long or is that never the case?  Does it count up to 63 (I
> am aware that the behaviour applied against 0 may be undefined) or does
> it just work on the low 32 bits?  (I assume the former)

The generic version checks sizeof(unsigned), so the former.
The GNU specific version using the builtin is meant to work with an unsigned
int and not a fixed width data type, so it's probably safe to assume it will.

> 
> Thanks
> 
> JC
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>