[FFmpeg-devel] [FFmpeg-soc] [PATCH] Separate video specific BufferRef properties into VideoProps

Stefano Sabatini stefano.sabatini-lala
Tue Aug 3 21:44:12 CEST 2010


On date Tuesday 2010-08-03 00:07:55 -0700, S.N. Hemanth Meenakshisundaram encoded:
> 
> Separated out video specific AVFilterBufferRef properties as requested
> and using a props void pointer. This passes make lavfitest and valgrind.
> 
> Regards,
> 
> ---
> 
>  ffmpeg.c                   |   14 ++++++++++----
>  ffplay.c                   |   22 ++++++++++++++++------
>  libavfilter/avfilter.c     |   15 +++++++++++++--
>  libavfilter/avfilter.h     |   43 ++++++++++++++++++++++++++++++++-----------
>  libavfilter/defaults.c     |   14 ++++++++------
>  libavfilter/vf_aspect.c    |    4 +++-
>  libavfilter/vf_crop.c      |    6 ++++--
>  libavfilter/vf_drawbox.c   |   10 +++++++---
>  libavfilter/vf_fifo.c      |    4 +++-
>  libavfilter/vf_fps.c       |    4 +++-
>  libavfilter/vf_hflip.c     |    4 +++-
>  libavfilter/vf_overlay.c   |   11 +++++++----
>  libavfilter/vf_pad.c       |    8 +++++---
>  libavfilter/vf_rotate.c    |   17 +++++++++++------
>  libavfilter/vf_scale.c     |    9 ++++++---
>  libavfilter/vf_transpose.c |   24 +++++++++++++++---------
>  libavfilter/vsrc_buffer.c  |   10 ++++++----
>  17 files changed, 152 insertions(+), 67 deletions(-)
> 
> 
> 

> diff --git a/ffmpeg.c b/ffmpeg.c
> index aec1f79..0cadaef 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -365,6 +365,7 @@ static int get_filtered_video_pic(AVFilterContext *ctx,
>                                    uint64_t *pts)
>  {
>      AVFilterBufferRef *pic;
> +    AVFilterBufferRefVideoProps *pic_props;
>  
>      if(avfilter_request_frame(ctx->inputs[0]))
>          return -1;
> @@ -372,13 +373,14 @@ static int get_filtered_video_pic(AVFilterContext *ctx,
>          return -1;
>      *picref = pic;
>      ctx->inputs[0]->cur_buf = NULL;
> +    AVFILTER_GET_REF_VIDEO_PROPS(pic_props, pic);

GET_BUFREF_VIDEO_PROPS -> more informative

>  
>      *pts          = pic->pts;
>  
>      memcpy(pic2->data,     pic->data,     sizeof(pic->data));
>      memcpy(pic2->linesize, pic->linesize, sizeof(pic->linesize));
> -    pic2->interlaced_frame = pic->interlaced;
> -    pic2->top_field_first  = pic->top_field_first;
> +    pic2->interlaced_frame = pic_props->interlaced;
> +    pic2->top_field_first  = pic_props->top_field_first;
>  
>      return 1;
>  }
> @@ -1680,8 +1682,11 @@ static int output_packet(AVInputStream *ist, int ist_index,
>          if (start_time == 0 || ist->pts >= start_time)
>  #if CONFIG_AVFILTER
>          while (frame_available) {
> -            if (ist->st->codec->codec_type == AVMEDIA_TYPE_VIDEO && ist->out_video_filter)
> +            AVFilterBufferRefVideoProps *pic_props = NULL;
> +            if (ist->st->codec->codec_type == AVMEDIA_TYPE_VIDEO && ist->out_video_filter) {
>                  get_filtered_video_pic(ist->out_video_filter, &ist->picref, &picture, &ist->pts);
> +                AVFILTER_GET_REF_VIDEO_PROPS(pic_props, ist->picref);
> +            }
>  #endif
>              for(i=0;i<nb_ostreams;i++) {
>                  int frame_size;
> @@ -1701,7 +1706,8 @@ static int output_packet(AVInputStream *ist, int ist_index,
>                              break;
>                          case AVMEDIA_TYPE_VIDEO:
>  #if CONFIG_AVFILTER
> -                            ost->st->codec->sample_aspect_ratio = ist->picref->pixel_aspect;
> +                            if (pic_props)
> +                                ost->st->codec->sample_aspect_ratio = pic_props->pixel_aspect;
>  #endif
>                              do_video_out(os, ost, ist, &picture, &frame_size);
>                              if (vstats_filename && frame_size)
> diff --git a/ffplay.c b/ffplay.c
> index c89a8b3..a4898c1 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -684,6 +684,9 @@ static void free_subpicture(SubPicture *sp)
>  static void video_image_display(VideoState *is)
>  {
>      VideoPicture *vp;
> +#if CONFIG_AVFILTER
> +    AVFilterBufferRefVideoProps *pic_props;
> +#endif
>      SubPicture *sp;
>      AVPicture pict;
>      float aspect_ratio;
> @@ -694,10 +697,11 @@ static void video_image_display(VideoState *is)
>      vp = &is->pictq[is->pictq_rindex];
>      if (vp->bmp) {
>  #if CONFIG_AVFILTER
> -         if (vp->picref->pixel_aspect.num == 0)
> +         AVFILTER_GET_REF_VIDEO_PROPS(pic_props, vp->picref);
> +         if (pic_props->pixel_aspect.num == 0)
>               aspect_ratio = 0;
>           else
> -             aspect_ratio = av_q2d(vp->picref->pixel_aspect);
> +             aspect_ratio = av_q2d(pic_props->pixel_aspect);
>  #else
>  
>          /* XXX: use variable in the frame */
> @@ -1561,6 +1565,7 @@ static int input_get_buffer(AVCodecContext *codec, AVFrame *pic)
>  {
>      AVFilterContext *ctx = codec->opaque;
>      AVFilterBufferRef  *ref;
> +    AVFilterBufferRefVideoProps *ref_props;
>      int perms = AV_PERM_WRITE;
>      int i, w, h, stride[4];
>      unsigned edge;
> @@ -1582,8 +1587,9 @@ static int input_get_buffer(AVCodecContext *codec, AVFrame *pic)
>      if(!(ref = avfilter_get_video_buffer(ctx->outputs[0], perms, w, h)))
>          return -1;
>  
> -    ref->w = codec->width;
> -    ref->h = codec->height;
> +    AVFILTER_GET_REF_VIDEO_PROPS(ref_props, ref);
> +    ref_props->w = codec->width;
> +    ref_props->h = codec->height;
>      for(i = 0; i < 4; i ++) {
>          unsigned hshift = (i == 1 || i == 2) ? av_pix_fmt_descriptors[ref->format].log2_chroma_w : 0;
>          unsigned vshift = (i == 1 || i == 2) ? av_pix_fmt_descriptors[ref->format].log2_chroma_h : 0;
> @@ -1610,13 +1616,15 @@ static void input_release_buffer(AVCodecContext *codec, AVFrame *pic)
>  static int input_reget_buffer(AVCodecContext *codec, AVFrame *pic)
>  {
>      AVFilterBufferRef *ref = pic->opaque;
> +    AVFilterBufferRefVideoProps *ref_props;
> +    AVFILTER_GET_REF_VIDEO_PROPS(ref_props, ref);
>  
>      if (pic->data[0] == NULL) {
>          pic->buffer_hints |= FF_BUFFER_HINTS_READABLE;
>          return codec->get_buffer(codec, pic);
>      }
>  
> -    if ((codec->width != ref->w) || (codec->height != ref->h) ||
> +    if ((codec->width != ref_props->w) || (codec->height != ref_props->h) ||
>          (codec->pix_fmt != ref->format)) {
>          av_log(codec, AV_LOG_ERROR, "Picture properties changed.\n");
>          return -1;
> @@ -1657,6 +1665,7 @@ static int input_request_frame(AVFilterLink *link)
>  {
>      FilterPriv *priv = link->src->priv;
>      AVFilterBufferRef *picref;
> +    AVFilterBufferRefVideoProps *pic_props;
>      int64_t pts = 0;
>      AVPacket pkt;
>      int ret;
> @@ -1674,10 +1683,11 @@ static int input_request_frame(AVFilterLink *link)
>                          picref->format, link->w, link->h);
>      }
>      av_free_packet(&pkt);
> +    AVFILTER_GET_REF_VIDEO_PROPS(pic_props, picref);
>  
>      picref->pts = pts;
>      picref->pos = pkt.pos;
> -    picref->pixel_aspect = priv->is->video_st->codec->sample_aspect_ratio;
> +    pic_props->pixel_aspect = priv->is->video_st->codec->sample_aspect_ratio;
>      avfilter_start_frame(link, picref);
>      avfilter_draw_slice(link, 0, link->h, 1);
>      avfilter_end_frame(link);
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 98a7204..24e7ea3 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -45,10 +45,16 @@ const char *avfilter_license(void)
>  #define link_dpad(link)     link->dst-> input_pads[link->dstpad]
>  #define link_spad(link)     link->src->output_pads[link->srcpad]
>  

> +#define AVFILTER_COPY_VIDEO_PROPS(dst, src) {\
> +    dst->props = av_malloc(sizeof(AVFilterBufferRefVideoProps));\
> +    memcpy(dst->props, src->props, sizeof(AVFilterBufferRefVideoProps));\
> +}

possibly avoid the use of macros used just once, they tend just to
obfuscate code.

>  AVFilterBufferRef *avfilter_ref_buffer(AVFilterBufferRef *ref, int pmask)
>  {
>      AVFilterBufferRef *ret = av_malloc(sizeof(AVFilterBufferRef));
>      *ret = *ref;
> +    AVFILTER_COPY_VIDEO_PROPS(ret, ref);
>      ret->perms &= pmask;
>      ret->buf->refcount ++;
>      return ret;
> @@ -58,6 +64,7 @@ void avfilter_unref_buffer(AVFilterBufferRef *ref)
>  {
>      if(!(--ref->buf->refcount))
>          ref->buf->free(ref->buf);
> +    av_free(ref->props);
>      av_free(ref);
>  }
>  
> @@ -173,13 +180,15 @@ int avfilter_config_links(AVFilterContext *filter)
>  
>  void ff_dprintf_picref(void *ctx, AVFilterBufferRef *picref, int end)
>  {
> +    AVFilterBufferRefVideoProps *pic_props;
> +    AVFILTER_GET_REF_VIDEO_PROPS(pic_props, picref);
>      dprintf(ctx,
>              "picref[%p data[%p, %p, %p, %p] linesize[%d, %d, %d, %d] pts:%"PRId64" pos:%"PRId64" a:%d/%d s:%dx%d]%s",
>              picref,
>              picref->data    [0], picref->data    [1], picref->data    [2], picref->data    [3],
>              picref->linesize[0], picref->linesize[1], picref->linesize[2], picref->linesize[3],
>              picref->pts, picref->pos,
> -            picref->pixel_aspect.num, picref->pixel_aspect.den, picref->w, picref->h,
> +            pic_props->pixel_aspect.num, pic_props->pixel_aspect.den, pic_props->w, pic_props->h,
>              end ? "\n" : "");
>  }
>  
> @@ -292,6 +301,7 @@ void avfilter_end_frame(AVFilterLink *link)
>  
>  void avfilter_draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
>  {
> +    AVFilterBufferRefVideoProps *cur_buf_props;
>      uint8_t *src[4], *dst[4];
>      int i, j, vsub;
>      void (*draw_slice)(AVFilterLink *, int, int, int);
> @@ -311,10 +321,11 @@ void avfilter_draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
>              } else
>                  src[i] = dst[i] = NULL;
>          }
> +        AVFILTER_GET_REF_VIDEO_PROPS(cur_buf_props, link->cur_buf);
>  
>          for(i = 0; i < 4; i ++) {
>              int planew =

> -                ff_get_plane_bytewidth(link->format, link->cur_buf->w, i);
> +                ff_get_plane_bytewidth(link->format, cur_buf_props->w, i);

note: this function should be moved to lavcore (and maybe cleaned up
using pixdescs).

>  
>              if(!src[i]) continue;
>  
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 96fca21..3ba85c8 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -37,6 +37,7 @@
>  #define LIBAVFILTER_BUILD       LIBAVFILTER_VERSION_INT
>  
>  #include <stddef.h>
> +#include <assert.h>
>  #include "libavcodec/avcodec.h"
>  
>  /**
> @@ -89,6 +90,21 @@ typedef struct AVFilterBuffer
>  #define AV_PERM_REUSE2   0x10   ///< can output the buffer multiple times, modified each time
>  
>  /**
> + * Video specific properties in a reference to an AVFilterBuffer. Since
> + * AVFilterBufferRef is common to different media formats, video specific
> + * per reference properties must be separated out.
> + */
> +
> +typedef struct AVFilterBufferRefVideoProps
> +{
> +    AVRational pixel_aspect;    ///< pixel aspect ratio
> +    int w;                      ///< image width
> +    int h;                      ///< image height
> +    int interlaced;             ///< is frame interlaced
> +    int top_field_first;        ///< field order
> +} AVFilterBufferRefVideoProps;
> +
> +/**
>   * A reference to an AVFilterBuffer. Since filters can manipulate the origin of
>   * a buffer to, for example, crop image without any memcpy, the buffer origin
>   * and dimensions are per-reference properties. Linesize is also useful for
> @@ -96,37 +112,42 @@ typedef struct AVFilterBuffer
>   *
>   * TODO: add anything necessary for frame reordering
>   */
> +
>  typedef struct AVFilterBufferRef
>  {
>      AVFilterBuffer *buf;        ///< the buffer that this is a reference to
>      uint8_t *data[4];           ///< picture data for each plane
>      int linesize[4];            ///< number of bytes per line
> -    int w;                      ///< image width
> -    int h;                      ///< image height
>      int format;                 ///< media format
>  
>      int64_t pts;                ///< presentation timestamp in units of 1/AV_TIME_BASE
>      int64_t pos;                ///< byte position in stream, -1 if unknown
>  
> -    AVRational pixel_aspect;    ///< pixel aspect ratio
> -
>      int perms;                  ///< permissions, see the AV_PERM_* flags
>  
> -    int interlaced;             ///< is frame interlaced
> -    int top_field_first;
> +    enum AVMediaType type;      ///< media type
> +    void *props;                ///< media props, cast it to the right type
>  } AVFilterBufferRef;
>  
> +#define AVFILTER_GET_REF_VIDEO_PROPS(props_ptr, ref) {\
> +    assert(ref->type == AVMEDIA_TYPE_VIDEO);\
> +    props_ptr = (AVFilterBufferRefVideoProps *)ref->props;\
> +}
> +
>  /**
>   * Copy properties of src to dst, without copying the actual video
>   * data.
>   */

>  static inline void avfilter_copy_bufref_props(AVFilterBufferRef *dst, AVFilterBufferRef *src)
>  {
> -    dst->pts             = src->pts;
> -    dst->pos             = src->pos;
> -    dst->pixel_aspect    = src->pixel_aspect;
> -    dst->interlaced      = src->interlaced;
> -    dst->top_field_first = src->top_field_first;
> +    AVFilterBufferRefVideoProps *props_src, *props_dst;
> +    AVFILTER_GET_REF_VIDEO_PROPS(props_src, src);
> +    AVFILTER_GET_REF_VIDEO_PROPS(props_dst, dst);
> +    dst->pts                   = src->pts;
> +    dst->pos                   = src->pos;
> +    props_dst->pixel_aspect    = props_src->pixel_aspect;
> +    props_dst->interlaced      = props_src->interlaced;
> +    props_dst->top_field_first = props_src->top_field_first;
>  }

mixing cosmetical and functional changes.

This can be outlined in a more readable way:

    // copy common props
    dst->pts                   = src->pts;
    dst->pos                   = src->pos;

    switch (src->type) {
    VIDEO:
    {
       AVFilterBufferRefVideoProps *src_props, *dst_props;
       AVFILTER_GET_REF_VIDEO_PROPS(src_props, src);
       AVFILTER_GET_REF_VIDEO_PROPS(dst_props, dst);
       *dst_props = *src_props;
    }
    ...

> diff --git a/libavfilter/defaults.c b/libavfilter/defaults.c
> index d607c31..36fe520 100644
> --- a/libavfilter/defaults.c
> +++ b/libavfilter/defaults.c
> @@ -36,12 +36,14 @@ AVFilterBufferRef *avfilter_default_get_video_buffer(AVFilterLink *link, int per
>  {
>      AVFilterBuffer *pic = av_mallocz(sizeof(AVFilterBuffer));
>      AVFilterBufferRef *ref = av_mallocz(sizeof(AVFilterBufferRef));
> +    AVFilterBufferRefVideoProps *ref_props = av_mallocz(sizeof(AVFilterBufferRefVideoProps));
>      int i, tempsize;
>      char *buf;
>  
> -    ref->buf   = pic;
> -    ref->w     = w;
> -    ref->h     = h;
> +    ref->buf         = pic;
> +    ref_props->w     = w;
> +    ref_props->h     = h;
> +    ref->props       = ref_props;
>  
>      /* make sure the buffer gets read permission or it's useless for output */
>      ref->perms = perms | AV_PERM_READ;
> @@ -49,15 +51,15 @@ AVFilterBufferRef *avfilter_default_get_video_buffer(AVFilterLink *link, int per
>      pic->refcount = 1;
>      ref->format   = link->format;
>      pic->free     = avfilter_default_free_buffer;
> -    av_fill_image_linesizes(pic->linesize, ref->format, ref->w);
> +    av_fill_image_linesizes(pic->linesize, ref->format, ref_props->w);
>  
>      for (i=0; i<4;i++)
>          pic->linesize[i] = FFALIGN(pic->linesize[i], 16);
>  
> -    tempsize = av_fill_image_pointers(pic->data, ref->format, ref->h, NULL, pic->linesize);
> +    tempsize = av_fill_image_pointers(pic->data, ref->format, ref_props->h, NULL, pic->linesize);
>      buf = av_malloc(tempsize + 16); // +2 is needed for swscaler, +16 to be
>                                      // SIMD-friendly
> -    av_fill_image_pointers(pic->data, ref->format, ref->h, buf, pic->linesize);
> +    av_fill_image_pointers(pic->data, ref->format, ref_props->h, buf, pic->linesize);
>  
>      memcpy(ref->data,     pic->data,     sizeof(pic->data));
>      memcpy(ref->linesize, pic->linesize, sizeof(pic->linesize));
> diff --git a/libavfilter/vf_aspect.c b/libavfilter/vf_aspect.c
> index bd18649..d466e05 100644
> --- a/libavfilter/vf_aspect.c
> +++ b/libavfilter/vf_aspect.c
> @@ -59,8 +59,10 @@ static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
>  static void start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>  {
>      AspectContext *aspect = link->dst->priv;
> +    AVFilterBufferRefVideoProps *pic_props;
> +    AVFILTER_GET_REF_VIDEO_PROPS(pic_props, picref);
>  
> -    picref->pixel_aspect = aspect->aspect;
> +    pic_props->pixel_aspect = aspect->aspect;
>      avfilter_start_frame(link->dst->outputs[0], picref);
>  }
>  
> diff --git a/libavfilter/vf_crop.c b/libavfilter/vf_crop.c
> index ddaf733..da6a618 100644
> --- a/libavfilter/vf_crop.c
> +++ b/libavfilter/vf_crop.c
> @@ -161,10 +161,12 @@ static void start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>  {
>      CropContext *crop = link->dst->priv;
>      AVFilterBufferRef *ref2 = avfilter_ref_buffer(picref, ~0);
> +    AVFilterBufferRefVideoProps *ref_props;
>      int i;
> +    AVFILTER_GET_REF_VIDEO_PROPS(ref_props, ref2);
>  
> -    ref2->w        = crop->w;
> -    ref2->h        = crop->h;
> +    ref_props->w        = crop->w;
> +    ref_props->h        = crop->h;
>  
>      ref2->data[0] += crop->y * ref2->linesize[0];
>      ref2->data[0] += (crop->x * crop->bpp) >> 3;

> diff --git a/libavfilter/vf_drawbox.c b/libavfilter/vf_drawbox.c

Post patches against latest SVN, otherwise that will be more work for
the committer. I'll fix the other filters in soc as I'll update it.

Rest of the patch looks fine to me.

Regards.
-- 
FFmpeg = Fundamental and Fabulous Magnificient Peaceful Energized God



More information about the ffmpeg-devel mailing list