[FFmpeg-devel] [PATCH v2] avfilter/formats: optimize ff_all_formats

[zhilizhao]

> On Dec 24, 2019, at 4:32 AM, Michael Niedermayer <michael at niedermayer.cc> wrote:
> 
> On Mon, Dec 23, 2019 at 04:11:53PM +0100, Nicolas George wrote:
>> Tomas Härdin (12019-12-09):
>>> As far as I can tell this is OK, and it passes FATE. But it just looks
>>> very very wrong. Why does this function even exist if all it
>>> effectively does is return the integers from 0..count-1?
>> 
>> For some time, there were gaps in the list of pixel formats, the task of
>> this function was precisely there to avoid the gaps and keep only the
>> actual pixel formats.
>> 
>> I am not sure about this change. It is technically valid, but it may
>> cause problems later.
>> 
>> There is an intermediate solution: keep the loop, but allocate only
>> once (simplified):
>> 
>>        while ((desc = ...))
>>            count++;
>>        ret->formats = av_malloc_array(count, sizeof(*ret->formats));
>>        count = 0;
>>        while ((desc = ...))
>>            ret->formats[count] = ...;
>> 
>> What do other developers think about it, with these explanations?

I tried this solution at first. The following reasons make me thought AV_PIX_FMT cannot have gaps.
Of course, can libavfilter depends on that is another question. I can switch to the robust solution
if you prefer.

1. AV_PIX_FMT_NB depends on !gaps, unless we calculate and set it manually

2. The reason to deprecated VAAPI in this way (ABI incompatible) is AV_PIX_FMT cannot have gaps

#if FF_API_VAAPI
    /** @name Deprecated pixel formats */
    /**@{*/
    AV_PIX_FMT_VAAPI_MOCO, ///< HW acceleration through VA API at motion compensation entry-point, Picture.data[3] contains a vaapi_render_state struct which contains macroblocks as well as various fields extracted from headers
    AV_PIX_FMT_VAAPI_IDCT, ///< HW acceleration through VA API at IDCT entry-point, Picture.data[3] contains a vaapi_render_state struct which contains fields extracted from headers
    AV_PIX_FMT_VAAPI_VLD,  ///< HW decoding through VA API, Picture.data[3] contains a VASurfaceID
    /**@}*/
    AV_PIX_FMT_VAAPI = AV_PIX_FMT_VAAPI_VLD,
#else
    /**
     *  Hardware acceleration through VA-API, data[3] contains a
     *  VASurfaceID.
     */
    AV_PIX_FMT_VAAPI,
#endif


> 
> The count once then allocate feels more robust than assuming on !gaps
> 
> Thanks
> 
> 
> [...]
> 
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> When you are offended at any man's fault, turn to yourself and study your
> own failings. Then you will forget your anger. -- Epictetus
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel>
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org <mailto:ffmpeg-devel-request at ffmpeg.org> with subject "unsubscribe".