Re: [FFmpeg-devel] [PATCH 2/2] avutil: make AVFrameSideData buffers ref-counted.

2015-03-25 Thread Michael Niedermayer 
On Mon, Mar 23, 2015 at 11:11:19AM -0400, Ronald S. Bultje wrote:
> ---
>  libavutil/frame.c | 53 ++---
>  libavutil/frame.h |  1 +
>  2 files changed, 39 insertions(+), 15 deletions(-)

applied

maybe someone should benchmark this for speed per size and
add a copy case where/if its faster

thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avutil: make AVFrameSideData buffers ref-counted.

2015-03-23 Thread Ronald S. Bultje 
Hi,

On Mon, Mar 23, 2015 at 12:16 PM, wm4  wrote:

> On Mon, 23 Mar 2015 11:11:19 -0400
> "Ronald S. Bultje"  wrote:
>
> > ---
> >  libavutil/frame.c | 53
> ++---
> >  libavutil/frame.h |  1 +
> >  2 files changed, 39 insertions(+), 15 deletions(-)
> >
> > diff --git a/libavutil/frame.c b/libavutil/frame.c
> > index 85f5637..4596927 100644
> > --- a/libavutil/frame.c
> > +++ b/libavutil/frame.c
> > @@ -115,7 +115,7 @@ static void free_side_data(AVFrameSideData **ptr_sd)
> >  {
> >  AVFrameSideData *sd = *ptr_sd;
> >
> > -av_freep(&sd->data);
> > +av_buffer_unref(&sd->buf);
> >  av_dict_free(&sd->metadata);
> >  av_freep(ptr_sd);
> >  }
> > @@ -275,7 +275,7 @@ int av_frame_get_buffer(AVFrame *frame, int align)
> >  return AVERROR(EINVAL);
> >  }
> >
> > -int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
> > +static int frame_copy_props(AVFrame *dst, const AVFrame *src, int
> force_copy)
> >  {
> >  int i;
> >
> > @@ -320,13 +320,28 @@ int av_frame_copy_props(AVFrame *dst, const
> AVFrame *src)
> >  if (   sd_src->type == AV_FRAME_DATA_PANSCAN
> >  && (src->width != dst->width || src->height != dst->height))
> >  continue;
> > -sd_dst = av_frame_new_side_data(dst, sd_src->type,
> > -sd_src->size);
> > -if (!sd_dst) {
> > -wipe_side_data(dst);
> > -return AVERROR(ENOMEM);
> > +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 {
> > +sd_dst = av_frame_new_side_data(dst, sd_src->type, 0);
> > +if (!sd_dst) {
> > +wipe_side_data(dst);
> > +return AVERROR(ENOMEM);
> > +}
> > +sd_dst->buf = av_buffer_ref(sd_src->buf);
> > +if (!sd_dst->buf) {
> > +wipe_side_data(dst);
> > +return AVERROR(ENOMEM);
> > +}
> > +sd_dst->data = sd_dst->buf->data;
> > +sd_dst->size = sd_dst->buf->size;
> >  }
> > -memcpy(sd_dst->data, sd_src->data, sd_src->size);
> >  av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
> >  }
> >
> > @@ -356,7 +371,7 @@ int av_frame_ref(AVFrame *dst, const AVFrame *src)
> >  dst->channel_layout = src->channel_layout;
> >  dst->nb_samples = src->nb_samples;
> >
> > -ret = av_frame_copy_props(dst, src);
> > +ret = frame_copy_props(dst, src, 0);
> >  if (ret < 0)
> >  return ret;
> >
> > @@ -530,6 +545,11 @@ int av_frame_make_writable(AVFrame *frame)
> >  return 0;
> >  }
> >
> > +int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
> > +{
> > +return frame_copy_props(dst, src, 1);
> > +}
> > +
> >  AVBufferRef *av_frame_get_plane_buffer(AVFrame *frame, int plane)
> >  {
> >  uint8_t *data;
> > @@ -580,13 +600,16 @@ AVFrameSideData *av_frame_new_side_data(AVFrame
> *frame,
> >  if (!ret)
> >  return NULL;
> >
> > -ret->data = av_malloc(size);
> > -if (!ret->data) {
> > -av_freep(&ret);
> > -return NULL;
> > -}
> > +if (size > 0) {
> > +ret->buf = av_buffer_alloc(size);
> > +if (!ret->buf) {
> > +av_freep(&ret);
> > +return NULL;
> > +}
> >
> > -ret->size = size;
> > +ret->data = ret->buf->data;
> > +ret->size = size;
> > +}
> >  ret->type = type;
> >
> >  frame->side_data[frame->nb_side_data++] = ret;
> > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 6b9ac6a..e65ad79 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -129,6 +129,7 @@ typedef struct AVFrameSideData {
> >  uint8_t *data;
> >  int  size;
> >  AVDictionary *metadata;
> > +AVBufferRef *buf;
> >  } AVFrameSideData;
> >
> >  /**
>
> What if someone wants to do write-accesses to the side-data? I don't
> think this patch handles this case.  av_frame_make_writable() should be
> extended.


av_frame_make_writable() copies (as opposed to refcounts) the side-data, so
that works fine. The refcount-increment code is only invoked for
av_frame_ref().

Ronald
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] avutil: make AVFrameSideData buffers ref-counted.

2015-03-23 Thread wm4 
On Mon, 23 Mar 2015 11:11:19 -0400
"Ronald S. Bultje"  wrote:

> ---
>  libavutil/frame.c | 53 ++---
>  libavutil/frame.h |  1 +
>  2 files changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index 85f5637..4596927 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -115,7 +115,7 @@ static void free_side_data(AVFrameSideData **ptr_sd)
>  {
>  AVFrameSideData *sd = *ptr_sd;
>  
> -av_freep(&sd->data);
> +av_buffer_unref(&sd->buf);
>  av_dict_free(&sd->metadata);
>  av_freep(ptr_sd);
>  }
> @@ -275,7 +275,7 @@ int av_frame_get_buffer(AVFrame *frame, int align)
>  return AVERROR(EINVAL);
>  }
>  
> -int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
> +static int frame_copy_props(AVFrame *dst, const AVFrame *src, int force_copy)
>  {
>  int i;
>  
> @@ -320,13 +320,28 @@ int av_frame_copy_props(AVFrame *dst, const AVFrame 
> *src)
>  if (   sd_src->type == AV_FRAME_DATA_PANSCAN
>  && (src->width != dst->width || src->height != dst->height))
>  continue;
> -sd_dst = av_frame_new_side_data(dst, sd_src->type,
> -sd_src->size);
> -if (!sd_dst) {
> -wipe_side_data(dst);
> -return AVERROR(ENOMEM);
> +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 {
> +sd_dst = av_frame_new_side_data(dst, sd_src->type, 0);
> +if (!sd_dst) {
> +wipe_side_data(dst);
> +return AVERROR(ENOMEM);
> +}
> +sd_dst->buf = av_buffer_ref(sd_src->buf);
> +if (!sd_dst->buf) {
> +wipe_side_data(dst);
> +return AVERROR(ENOMEM);
> +}
> +sd_dst->data = sd_dst->buf->data;
> +sd_dst->size = sd_dst->buf->size;
>  }
> -memcpy(sd_dst->data, sd_src->data, sd_src->size);
>  av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
>  }
>  
> @@ -356,7 +371,7 @@ int av_frame_ref(AVFrame *dst, const AVFrame *src)
>  dst->channel_layout = src->channel_layout;
>  dst->nb_samples = src->nb_samples;
>  
> -ret = av_frame_copy_props(dst, src);
> +ret = frame_copy_props(dst, src, 0);
>  if (ret < 0)
>  return ret;
>  
> @@ -530,6 +545,11 @@ int av_frame_make_writable(AVFrame *frame)
>  return 0;
>  }
>  
> +int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
> +{
> +return frame_copy_props(dst, src, 1);
> +}
> +
>  AVBufferRef *av_frame_get_plane_buffer(AVFrame *frame, int plane)
>  {
>  uint8_t *data;
> @@ -580,13 +600,16 @@ AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
>  if (!ret)
>  return NULL;
>  
> -ret->data = av_malloc(size);
> -if (!ret->data) {
> -av_freep(&ret);
> -return NULL;
> -}
> +if (size > 0) {
> +ret->buf = av_buffer_alloc(size);
> +if (!ret->buf) {
> +av_freep(&ret);
> +return NULL;
> +}
>  
> -ret->size = size;
> +ret->data = ret->buf->data;
> +ret->size = size;
> +}
>  ret->type = type;
>  
>  frame->side_data[frame->nb_side_data++] = ret;
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 6b9ac6a..e65ad79 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -129,6 +129,7 @@ typedef struct AVFrameSideData {
>  uint8_t *data;
>  int  size;
>  AVDictionary *metadata;
> +AVBufferRef *buf;
>  } AVFrameSideData;
>  
>  /**

What if someone wants to do write-accesses to the side-data? I don't
think this patch handles this case.  av_frame_make_writable() should be
extended.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/2] avutil: make AVFrameSideData buffers ref-counted.

2015-03-23 Thread Ronald S. Bultje 
---
 libavutil/frame.c | 53 ++---
 libavutil/frame.h |  1 +
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 85f5637..4596927 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -115,7 +115,7 @@ static void free_side_data(AVFrameSideData **ptr_sd)
 {
 AVFrameSideData *sd = *ptr_sd;
 
-av_freep(&sd->data);
+av_buffer_unref(&sd->buf);
 av_dict_free(&sd->metadata);
 av_freep(ptr_sd);
 }
@@ -275,7 +275,7 @@ int av_frame_get_buffer(AVFrame *frame, int align)
 return AVERROR(EINVAL);
 }
 
-int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
+static int frame_copy_props(AVFrame *dst, const AVFrame *src, int force_copy)
 {
 int i;
 
@@ -320,13 +320,28 @@ int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
 if (   sd_src->type == AV_FRAME_DATA_PANSCAN
 && (src->width != dst->width || src->height != dst->height))
 continue;
-sd_dst = av_frame_new_side_data(dst, sd_src->type,
-sd_src->size);
-if (!sd_dst) {
-wipe_side_data(dst);
-return AVERROR(ENOMEM);
+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 {
+sd_dst = av_frame_new_side_data(dst, sd_src->type, 0);
+if (!sd_dst) {
+wipe_side_data(dst);
+return AVERROR(ENOMEM);
+}
+sd_dst->buf = av_buffer_ref(sd_src->buf);
+if (!sd_dst->buf) {
+wipe_side_data(dst);
+return AVERROR(ENOMEM);
+}
+sd_dst->data = sd_dst->buf->data;
+sd_dst->size = sd_dst->buf->size;
 }
-memcpy(sd_dst->data, sd_src->data, sd_src->size);
 av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);
 }
 
@@ -356,7 +371,7 @@ int av_frame_ref(AVFrame *dst, const AVFrame *src)
 dst->channel_layout = src->channel_layout;
 dst->nb_samples = src->nb_samples;
 
-ret = av_frame_copy_props(dst, src);
+ret = frame_copy_props(dst, src, 0);
 if (ret < 0)
 return ret;
 
@@ -530,6 +545,11 @@ int av_frame_make_writable(AVFrame *frame)
 return 0;
 }
 
+int av_frame_copy_props(AVFrame *dst, const AVFrame *src)
+{
+return frame_copy_props(dst, src, 1);
+}
+
 AVBufferRef *av_frame_get_plane_buffer(AVFrame *frame, int plane)
 {
 uint8_t *data;
@@ -580,13 +600,16 @@ AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
 if (!ret)
 return NULL;
 
-ret->data = av_malloc(size);
-if (!ret->data) {
-av_freep(&ret);
-return NULL;
-}
+if (size > 0) {
+ret->buf = av_buffer_alloc(size);
+if (!ret->buf) {
+av_freep(&ret);
+return NULL;
+}
 
-ret->size = size;
+ret->data = ret->buf->data;
+ret->size = size;
+}
 ret->type = type;
 
 frame->side_data[frame->nb_side_data++] = ret;
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 6b9ac6a..e65ad79 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -129,6 +129,7 @@ typedef struct AVFrameSideData {
 uint8_t *data;
 int  size;
 AVDictionary *metadata;
+AVBufferRef *buf;
 } AVFrameSideData;
 
 /**
-- 
2.1.2

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel