[FFmpeg-devel] [PATCH 1/5] libavutil: add convenience accessors for frame side data

[Lynne]

Apr 30, 2021, 13:34 by bradh at frogmouth.net:

> Signed-off-by: Brad Hards <bradh at frogmouth.net>
> ---
>  libavutil/frame.c | 31 +++++++++++++++++++++++++++++++
>  libavutil/frame.h | 23 +++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 2ec59b44b1..9f9953c2b4 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -625,6 +625,37 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
>  return NULL;
>  }
>  
> +AVFrameSideData *av_frame_get_side_data_n(const AVFrame *frame,
> +                                          enum AVFrameSideDataType type,
> +                                          const int side_data_instance)
> +{
> +    int i;
> +    int n = 0;
> +
> +    for (i = 0; i < frame->nb_side_data; i++) {
> +        if (frame->side_data[i]->type == type) {
> +            if (n == side_data_instance) {
> +                return frame->side_data[i];
> +            } else {
> +                n++;
> +            }
> +        }
> +    }
> +    return NULL;
> +}
>

_follow_the_code_style_
We have a document! We have thousands of lines of code already written with it!
We do not add brackets on one-line statements, and we allow for (int loops.

I don't like this function's name, and I don't like the way it operates.
If we do allow multiple side-data entries with the same type (my opinion is if you can't read them
without workarounds they're just not allowed), I'd much rather have a linked list, which would
allow us to keep the same style of _next(prev) iteration functions we've used everywhere else.
Plus, it would mean you don't have to do a worst possible case iteration for each lookup when you
have a million side data entries. Users have wanted to do this.

I think we should just have a av_frame_get_side_data_next(prev), where prev will be
NULL for the first call. Users can filter by data type themselves.
That way av_frame_get_side_data_nb can be removed.

I'd also like an APIchanges entry we're allowing multiple side data entries with the same type.
This is not a small change in behavior that we're making official.