[FFmpeg-devel] [PATCH] frame: add an av_frame_new_side_data_from_buf function

[James Almer]

On 10/16/2017 12:02 PM, Rostislav Pehlivanov wrote:
> Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
> ---
>  libavutil/frame.c | 16 ++++++----------
>  libavutil/frame.h | 13 +++++++++++++
>  2 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index d5fd2932e3..0668c888ea 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -26,11 +26,6 @@
>  #include "mem.h"
>  #include "samplefmt.h"
>  
> -
> -static AVFrameSideData *frame_new_side_data(AVFrame *frame,
> -                                            enum AVFrameSideDataType type,
> -                                            AVBufferRef *buf);
> -
>  MAKE_ACCESSORS(AVFrame, frame, int64_t, best_effort_timestamp)
>  MAKE_ACCESSORS(AVFrame, frame, int64_t, pkt_duration)
>  MAKE_ACCESSORS(AVFrame, frame, int64_t, pkt_pos)
> @@ -356,7 +351,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              }
>              memcpy(sd_dst->data, sd_src->data, sd_src->size);
>          } else {
> -            sd_dst = frame_new_side_data(dst, sd_src->type, av_buffer_ref(sd_src->buf));
> +            sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type,
> +                                                     av_buffer_ref(sd_src->buf));
>              if (!sd_dst) {
>                  wipe_side_data(dst);
>                  return AVERROR(ENOMEM);
> @@ -636,9 +632,9 @@ AVBufferRef *av_frame_get_plane_buffer(AVFrame *frame, int plane)
>      return NULL;
>  }
>  
> -static AVFrameSideData *frame_new_side_data(AVFrame *frame,
> -                                            enum AVFrameSideDataType type,
> -                                            AVBufferRef *buf)
> +AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame *frame,
> +                                                 enum AVFrameSideDataType type,
> +                                                 AVBufferRef *buf)
>  {
>      AVFrameSideData *ret, **tmp;
>  
> @@ -676,7 +672,7 @@ AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
>                                          int size)
>  {
>  
> -    return frame_new_side_data(frame, type, av_buffer_alloc(size));
> +    return av_frame_new_side_data_from_buf(frame, type, av_buffer_alloc(size));
>  }
>  
>  AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index abe4f4fd17..1430d8363f 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -762,6 +762,19 @@ AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
>                                          enum AVFrameSideDataType type,
>                                          int size);
>  
> +/**
> + * Add a new side data to a frame from an existing AVBufferRef
> + *
> + * @param frame a frame to which the side data should be added
> + * @param type type of the added side data
> + * @param a valid AVBufferRef
> + *
> + * @return newly added side data on success, NULL on error
> + */
> +AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame *frame,
> +                                                 enum AVFrameSideDataType type,
> +                                                 AVBufferRef *buf);
> +
>  /**
>   * @return a pointer to the side data of a given type on success, NULL if there
>   * is no side data with such type in this frame.

Is there a case where an AVBufferRef would be pre-allocated instead of
created specifically to be used with this function?
I ask because i think it would be better to instead make the function
create a frame side data the same way av_packet_add_side_data() and
av_stream_add_side_data() do, letting said functions alloc the
AVBufferRef, if anything to be consistent across all side data APIs.