[FFmpeg-devel] [RFC PATCH] avutil/frame: fix remove_side_data

[Marton Balint]

On Sun, 3 Nov 2019, Zhao Zhili wrote:

> From: Zhao Zhili <zhilizhao at tencent.com>
>
> remove_side_data is supposed to remove a single instance by design.
> Since new_side_data() doesn't forbid add multiple instance of the
> same type, remove_side_data should deal with that.

Please update the docs of the function that it removes all side data 
instances of the specified type.

> ---
> libavutil/frame.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index dcf1fc3d17..10d06dd29f 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -805,15 +805,20 @@ int av_frame_copy(AVFrame *dst, const AVFrame *src)
> void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type)
> {
>     int i;
> -
> -    for (i = 0; i < frame->nb_side_data; i++) {
> -        AVFrameSideData *sd = frame->side_data[i];
> -        if (sd->type == type) {
> -            free_side_data(&frame->side_data[i]);
> -            frame->side_data[i] = frame->side_data[frame->nb_side_data - 1];
> -            frame->nb_side_data--;
> +    int found;
> +
> +    do {
> +        found = 0;
> +        for (i = 0; i < frame->nb_side_data; i++) {
> +            AVFrameSideData *sd = frame->side_data[i];
> +            if (sd->type == type) {
> +                free_side_data(&frame->side_data[i]);
> +                frame->side_data[i] = frame->side_data[frame->nb_side_data - 1];
> +                frame->nb_side_data--;
> +                found = 1;
> +            }
>         }
> -    }
> +    } while (found);

I am sure this can be done in a single loop. (E.g.: decrement i when side 
data is found)

Regards,
Marton