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

[wm4]

On Mon, 16 Oct 2017 16:24:56 +0100
Rostislav Pehlivanov <atomnuker at gmail.com> wrote:

> On 16 October 2017 at 16:14, wm4 <nfxjfg at googlemail.com> wrote:
> 
> > On Mon, 16 Oct 2017 16:02:19 +0100
> > Rostislav Pehlivanov <atomnuker at gmail.com> 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.  
> >
> > LGTM, but doesn't document that it unrefs the buf on error. (We
> > probably want to change those semantics anyway?)
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >  
> 
> 
> Changed locally:
> 
> >- * @param a valid AVBufferRef
> >+ * @param a valid AVBufferRef, unreferenced on error  

Also could say that AVBufferRef changes ownership to the AVFrame on
success. Sorry for not mentioning this earlier.