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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2743 bytes --]

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


> 
> The other proposals seem significantly more complex and fragile than
> this one.

i agree too, though allowing av_pix_fmt_desc_get() to be done at compile
time would allow its use in inlined functions and macros much broader
so these are not 1:1 comparable as they serve only overlapping goals

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".