[FFmpeg-devel] [PATCH v2 1/1] avcodec/vpp_qsv: Copy side data from input to output frame

[Anton Khirnov]

Quoting Soft Works (2021-11-30 15:22:38)
> Signed-off-by: softworkz <softworkz at hotmail.com>
> ---
> V2: Add public method av_frame_copy_side_data() instead to copying the implementation.
> 
>  libavfilter/qsvvpp.c         |  5 ++++
>  libavfilter/vf_overlay_qsv.c | 19 +++++++++---
>  libavutil/frame.c            | 57 ++++++++++++++++++++----------------
>  libavutil/frame.h            | 12 ++++++++
>  4 files changed, 64 insertions(+), 29 deletions(-)

This should be split: new API in one commit, using it in the second.
New API also needs a minor version bump and a doc/APIchanges entry.

> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 1f1f573407..7a19245ea4 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -296,6 +296,36 @@ int av_frame_get_buffer2(AVFrame *frame, int align)
>      }
>  }
>  
> +int av_frame_copy_side_data(AVFrame* dst, const AVFrame* src, int force_copy)
> +{
> +    for (unsigned i = 0; i < src->nb_side_data; i++) {
> +        const AVFrameSideData *sd_src = src->side_data[i];
> +        AVFrameSideData *sd_dst;
> +        if (   sd_src->type == AV_FRAME_DATA_PANSCAN
> +            && (src->width != dst->width || src->height != dst->height))
> +            continue;

Yuck. I know it's already there, but it really really shouldn't be. This
function does not have enough information to make such decisions.

> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 9dfd5a886a..6dec040ce8 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -885,6 +885,18 @@ int av_frame_copy(AVFrame *dst, const AVFrame *src);
>   */
>  int av_frame_copy_props(AVFrame *dst, const AVFrame *src);
>  
> +/**
> + * Copy only side-data from src to dst.
> + *
> + * @param dst a frame to which the side data should be copied.
> + * @param src a frame from which to copy the side data.
> + * @param force_copy determines whether to copy the actual data or only just
> + *        create references to the buffers.
> + *
> + * @return >= 0 on success, a negative AVERROR on error.
> + */
> +int av_frame_copy_side_data(AVFrame* dst, const AVFrame* src, int force_copy);

Better change force_copy into unsigned int flags and define e.g.
AV_FRAME_COPY_PROPS_REF to mean you want refs rather than copies. That
way you're not wasting an entire integer argument on a single flag and
allowing future extensions.

-- 
Anton Khirnov