[FFmpeg-devel] [PATCH] avutil/frame: Add av_frame_transfer_side_data() function

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Tue Dec 7 13:28:09 EET 2021


Anton Khirnov:
> From: Soft Works <softworkz at hotmail.com>
> 
> Signed-off-by: softworkz <softworkz at hotmail.com>
> Signed-off-by: Anton Khirnov <anton at khirnov.net>
> ---
>  doc/APIchanges      |  4 +++
>  libavutil/frame.c   | 63 ++++++++++++++++++++++++++-------------------
>  libavutil/frame.h   | 20 ++++++++++++++
>  libavutil/version.h |  4 +--
>  4 files changed, 63 insertions(+), 28 deletions(-)
> 
> - renamed the function to av_frame_transfer_side_data(), since actually
>   copying side data is only one of the two possible modes of behavior
>   (and not even the default one)
> - renamed flags accordingly
> - added the AV_FRAME_TRANSFER_SD_FILTER flags as discussed
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 2914ad6734..79cfea00b8 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,10 @@ libavutil:     2021-04-27
>  
>  API changes, most recent first:
>  
> +2021-12-02 - xxxxxxxxxx - lavu 57.11.100 - frame.h
> +  Add av_frame_transfer_side_data(), AV_FRAME_TRANSFER_SD_COPY, and
> +  AV_FRAME_TRANSFER_SD_FILTER.
> +
>  2021-11-xx - xxxxxxxxxx - lavfi 8.19.100 - avfilter.h
>    Add AVFILTER_FLAG_METADATA_ONLY.
>  
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 0912ad9131..0b087cc4c9 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -253,9 +253,40 @@ int av_frame_get_buffer(AVFrame *frame, int align)
>      return AVERROR(EINVAL);
>  }
>  
> +int av_frame_transfer_side_data(AVFrame *dst, const AVFrame *src, int flags)
> +{
> +    for (unsigned i = 0; i < src->nb_side_data; i++) {
> +        const AVFrameSideData *sd_src = src->side_data[i];
> +        AVFrameSideData *sd_dst;
> +        if ((flags & AV_FRAME_TRANSFER_SD_FILTER) &&
> +            sd_src->type == AV_FRAME_DATA_PANSCAN          &&

Weird whitespace.

> +            (src->width != dst->width || src->height != dst->height))
> +            continue;
> +        if (flags & AV_FRAME_TRANSFER_SD_COPY) {
> +            sd_dst = av_frame_new_side_data(dst, sd_src->type,
> +                                            sd_src->size);
> +            if (!sd_dst) {
> +                wipe_side_data(dst);
> +                return AVERROR(ENOMEM);
> +            }
> +            memcpy(sd_dst->data, sd_src->data, sd_src->size);
> +        } else {
> +            AVBufferRef *ref = av_buffer_ref(sd_src->buf);
> +            sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type, ref);
> +            if (!sd_dst) {
> +                av_buffer_unref(&ref);
> +                wipe_side_data(dst);
> +                return AVERROR(ENOMEM);
> +            }
> +        }
> +        av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
> +    }
> +    return 0;
> +}
> +
>  static int frame_copy_props(AVFrame *dst, const AVFrame *src, int force_copy)
>  {
> -    int ret, i;
> +    int ret;
>  
>      dst->key_frame              = src->key_frame;
>      dst->pict_type              = src->pict_type;
> @@ -291,31 +322,11 @@ static int frame_copy_props(AVFrame *dst, const AVFrame *src, int force_copy)
>  
>      av_dict_copy(&dst->metadata, src->metadata, 0);
>  
> -    for (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;
> -        if (force_copy) {
> -            sd_dst = av_frame_new_side_data(dst, sd_src->type,
> -                                            sd_src->size);
> -            if (!sd_dst) {
> -                wipe_side_data(dst);
> -                return AVERROR(ENOMEM);
> -            }
> -            memcpy(sd_dst->data, sd_src->data, sd_src->size);
> -        } else {
> -            AVBufferRef *ref = av_buffer_ref(sd_src->buf);
> -            sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type, ref);
> -            if (!sd_dst) {
> -                av_buffer_unref(&ref);
> -                wipe_side_data(dst);
> -                return AVERROR(ENOMEM);
> -            }
> -        }
> -        av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
> -    }
> +    ret = av_frame_transfer_side_data(dst, src,
> +            (force_copy ? AV_FRAME_TRANSFER_SD_COPY : 0) |
> +            AV_FRAME_TRANSFER_SD_FILTER);
> +    if (ret < 0)
> +        return ret;
>  
>      ret = av_buffer_replace(&dst->opaque_ref, src->opaque_ref);
>      ret |= av_buffer_replace(&dst->private_ref, src->private_ref);
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 3f295f6b9e..deb399f1da 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -873,6 +873,26 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
>   */
>  void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type);
>  
> +/**
> + * Copy side data, rather than creating new references.
> + */
> +#define AV_FRAME_TRANSFER_SD_COPY      (1 << 0)
> +/**
> + * Filter out side data that does not match dst properties.
> + */
> +#define AV_FRAME_TRANSFER_SD_FILTER    (1 << 1)
> +
> +/**
> + * Transfer all side-data from src to dst.
> + *
> + * @param flags a combination of AV_FRAME_TRANSFER_SD_*
> + *
> + * @return >= 0 on success, a negative AVERROR on error.
> + *
> + * @note This function will create new references to side data buffers in src,
> + * unless the AV_FRAME_TRANSFER_SD_COPY flag is passed.
> + */
> +int av_frame_transfer_side_data(AVFrame *dst, const AVFrame *src, int flags);
>  

I wonder whether this should be side-data only. The fields of an AVFrame
can be divided into several groups; we currently divide it into the
actual data fields and props and with this function props would be
further subdivided into side-data and non-side-data. We have functions
for copying some of these, but not for all; there is e.g. no function
that performs a copy of frame data (whether shallow or deep) and only
that. We also have no functions for moving or unref/resetting one group
of data.  If there were a flag for every group (plus another flag for
whether one wants deep or shallow copies (if that makes sense at all)),
one could reuse the same function. If there were
av_frame_make_writable() could be as follows in the non-writable case:
Allocate buffers in tmp frame, copy data into them, unref data stuff
from src frame and move data from tmp to src frame. (Whereas the current
implementation copies side-data (it is even an avoidable deep copy).)

(I am of course aware that the signature of this function would allow to
extend it to more general copying, it is just that the name would be
inappropriate.)

>  /**
>   * Flags for frame cropping.
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 017fc277a6..0e7b36865a 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -79,8 +79,8 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  57
> -#define LIBAVUTIL_VERSION_MINOR  10
> -#define LIBAVUTIL_VERSION_MICRO 101
> +#define LIBAVUTIL_VERSION_MINOR  11
> +#define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
>                                                 LIBAVUTIL_VERSION_MINOR, \
> 



More information about the ffmpeg-devel mailing list