[FFmpeg-devel] [PATCH 2/2] swscale/input: Avoid calls to av_pix_fmt_desc_get()

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Sun Sep 18 23:58:02 EEST 2022


Michael Niedermayer:
> On Fri, Sep 16, 2022 at 09:00:24AM +0200, Anton Khirnov wrote:
>> Quoting Andreas Rheinhardt (2022-09-09 20:15:22)
>>> Michael Niedermayer:
>>>> On Thu, Sep 08, 2022 at 11:44:51PM +0200, Andreas Rheinhardt wrote:
>>>>> Michael Niedermayer:
>>>>>> On Thu, Sep 08, 2022 at 09:38:51PM +0200, Andreas Rheinhardt wrote:
>>>>>>> Michael Niedermayer:
>>>> [...]
>>>>>> To me if i look at the evolution
>>>>>> of isBE() / code checking BE-ness it become more messy over time
>>>>>>
>>>>>> I think it would be interresting to think about if we can make
>>>>>> av_pix_fmt_desc_get(compile time constant) work at compile time.
>>>>>> or if we maybe can return to a simpler implementation
>>>>>>
>>>>>
>>>>> We could put the av_pix_fmt_descriptors array into an internal header
>>>>> and use something like
>>>>>
>>>>> static av_always_inline const AVPixFmtDescriptor
>>>>> *ff_pix_fmt_descriptor_get(enum AVPixelFormat fmt)
>>>>> {
>>>>>     if (av_builtin_constant_p(fmt))
>>>>>         return &av_pix_fmt_descriptors[fmt];
>>>>>     return av_pix_fmt_desc_get(fmt);
>>>>> }
>>>>
>>>> yes thats what i was thinking of too.
>>>>
>>>
>>> Seems like Anton is away for a week or so. I am sure he has an opinion
>>> on the above approach. I think we will wait for him or shall I apply the
>>> patches as they are given that they do not block any later alternative
>>> solution?
>>> (There is one thing I already don't like about the alternative solution:
>>> It relies on av_builtin_constant_p, which not every compiler supports.)
>>
>> For what my opinion is worth, the patch as is with some extra
>> explanatory comments for the new IS_BE* macros seems like the best
>> solution to me. They are indeed slightly confusing at first glance, but
>> quite obvious if you look at the code for two minutes (less for people
>> smarter than me). And I think few people will need to understand how
>> precisely they work anyway.
> 
> i agree, maybe we can solve this by changing the IS_BE* names instead
> of adding comments.
> IS_BE, ENDIAN_IDENTIFER, 
> (0,1)  , (LE, BE)
> 
> or something like that, may reduce the need for comments
> 
> 

I sent a version 2 here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-September/301535.html Can
you take a look at it, please?

- Andreas


More information about the ffmpeg-devel mailing list