[FFmpeg-devel] [FFmpeg-soc] [PATCH] Separate video specific BufferRef properties into VideoProps
Stefano Sabatini
stefano.sabatini-lala
Sun Aug 8 00:26:44 CEST 2010
On date Wednesday 2010-08-04 07:37:33 -0700, S.N. Hemanth Meenakshisundaram encoded:
> On 08/03/2010 12:44 PM, Stefano Sabatini wrote:
> > On date Tuesday 2010-08-03 00:07:55 -0700, S.N. Hemanth Meenakshisundaram encoded:
> >
> >
> > GET_BUFREF_VIDEO_PROPS -> more informative
> >
>
> Done.
> > possibly avoid the use of macros used just once, they tend just to
> > obfuscate code.
> >
> >
>
> Removed
>
> >
> >> - 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).
> >
>
> Will do this in a separate patch.
>
> >
> > 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;
> > }
> >
>
> Changed as suggested.
>
> >> 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.
> >
> >
> >
>
> This patch is now against latest SVN and passes lavfitest.
>
> diff --git a/ffmpeg.c b/ffmpeg.c
> index aec1f79..07017ed 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_BUFREF_VIDEO_PROPS(pic_props, pic);
>
> *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_BUFREF_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..268bfa1 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_BUFREF_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_BUFREF_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_BUFREF_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_BUFREF_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..f9cad7c 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -45,10 +45,17 @@ 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));\
> +}
> +
> AVFilterBufferRef *avfilter_ref_buffer(AVFilterBufferRef *ref, int pmask)
> {
> AVFilterBufferRef *ret = av_malloc(sizeof(AVFilterBufferRef));
> *ret = *ref;
> + ret->props = av_malloc(sizeof(AVFilterBufferRefVideoProps));
> + memcpy(ret->props, ref->props, sizeof(AVFilterBufferRefVideoProps));
> ret->perms &= pmask;
> ret->buf->refcount ++;
> return ret;
I suppose #define AVFILTER_COPY_VIDEO_PROPS(dst, src) is not anymore
required.
> @@ -58,6 +65,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 +181,15 @@ int avfilter_config_links(AVFilterContext *filter)
>
> void ff_dprintf_picref(void *ctx, AVFilterBufferRef *picref, int end)
> {
> + AVFilterBufferRefVideoProps *pic_props;
possibly call this picref_props
> + AVFILTER_GET_BUFREF_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 +302,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 +322,11 @@ void avfilter_draw_slice(AVFilterLink *link, int y, int h, int slice_dir)
> } else
> src[i] = dst[i] = NULL;
> }
> + AVFILTER_GET_BUFREF_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);
>
> if(!src[i]) continue;
>
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index 4f80634..e7e3105 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
> @@ -101,34 +117,41 @@ 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
Can format be moved to AVFilterBufferRefVideoProps? (Eventually in a
separate patch, no high priority).
> 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 of buffer data
> + void *props; ///< media specific properties, cast to right type
> } AVFilterBufferRef;
>
> +#define AVFILTER_GET_BUFREF_VIDEO_PROPS(props_ptr, ref) {\
> + assert(ref->type == AVMEDIA_TYPE_VIDEO);\
> + props_ptr = (AVFilterBufferRefVideoProps *)ref->props;\
> +}
Sorry, maybe AVFILTER_GET_BUFFER_REF_VIDEO_PROPS is even better.
> +
> /**
> * Copy properties of src to dst, without copying the actual video
> * data.
> */
> static inline void avfilter_copy_bufref_props(AVFilterBufferRef *dst, AVFilterBufferRef *src)
> {
> + // copy common properties
> 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;
> - dst->w = src->w;
> - dst->h = src->h;
> +
> + switch (src->type) {
> + case AVMEDIA_TYPE_VIDEO:
> + {
> + AVFilterBufferRefVideoProps *src_props, *dst_props;
> + AVFILTER_GET_BUFREF_VIDEO_PROPS(src_props, src);
> + AVFILTER_GET_BUFREF_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..dfb4220 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_BUFREF_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 704458f..35f0263 100644
> --- a/libavfilter/vf_crop.c
> +++ b/libavfilter/vf_crop.c
> @@ -133,10 +133,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_BUFREF_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->max_step[0]);
> diff --git a/libavfilter/vf_pad.c b/libavfilter/vf_pad.c
> index 24ff034..77eb5dc 100644
> --- a/libavfilter/vf_pad.c
> +++ b/libavfilter/vf_pad.c
> @@ -427,9 +427,11 @@ static int color_request_frame(AVFilterLink *link)
> {
> ColorContext *color = link->src->priv;
> AVFilterBufferRef *picref = avfilter_get_video_buffer(link, AV_PERM_WRITE, color->w, color->h);
> - picref->pixel_aspect = (AVRational) {1, 1};
> - picref->pts = av_rescale_q(color->pts++, color->time_base, AV_TIME_BASE_Q);
> - picref->pos = 0;
> + AVFilterBufferRefVideoProps *pic_props;
> + AVFILTER_GET_BUFREF_VIDEO_PROPS(pic_props, picref);
> + pic_props->pixel_aspect = (AVRational) {1, 1};
> + picref->pts = av_rescale_q(color->pts++, color->time_base, AV_TIME_BASE_Q);
> + picref->pos = 0;
>
> avfilter_start_frame(link, avfilter_ref_buffer(picref, ~0));
> draw_rectangle(picref,
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 0486841..172ea27 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -147,18 +147,21 @@ static void start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
> ScaleContext *scale = link->dst->priv;
> AVFilterLink *outlink = link->dst->outputs[0];
> AVFilterBufferRef *outpicref;
> + AVFilterBufferRefVideoProps *pic_props, *outpic_props;
> + AVFILTER_GET_BUFREF_VIDEO_PROPS(pic_props, picref);
>
> scale->hsub = av_pix_fmt_descriptors[link->format].log2_chroma_w;
> scale->vsub = av_pix_fmt_descriptors[link->format].log2_chroma_h;
>
> outpicref = avfilter_get_video_buffer(outlink, AV_PERM_WRITE, outlink->w, outlink->h);
> + AVFILTER_GET_BUFREF_VIDEO_PROPS(outpic_props, outpicref);
> avfilter_copy_bufref_props(outpicref, picref);
>
> outlink->out_buf = outpicref;
>
> - av_reduce(&outpicref->pixel_aspect.num, &outpicref->pixel_aspect.den,
> - (int64_t)picref->pixel_aspect.num * outlink->h * link->w,
> - (int64_t)picref->pixel_aspect.den * outlink->w * link->h,
> + av_reduce(&outpic_props->pixel_aspect.num, &outpic_props->pixel_aspect.den,
> + (int64_t)pic_props->pixel_aspect.num * outlink->h * link->w,
> + (int64_t)pic_props->pixel_aspect.den * outlink->w * link->h,
> INT_MAX);
>
> scale->slice_y = 0;
> diff --git a/libavfilter/vsrc_buffer.c b/libavfilter/vsrc_buffer.c
> index 2bec911..c04e0f0 100644
> --- a/libavfilter/vsrc_buffer.c
> +++ b/libavfilter/vsrc_buffer.c
> @@ -106,6 +106,7 @@ static int request_frame(AVFilterLink *link)
> {
> BufferSourceContext *c = link->src->priv;
> AVFilterBufferRef *picref;
> + AVFilterBufferRefVideoProps *pic_props;
>
> if (!c->has_frame) {
> av_log(link->src, AV_LOG_ERROR,
> @@ -118,14 +119,15 @@ static int request_frame(AVFilterLink *link)
> picref = avfilter_get_video_buffer(link, AV_PERM_WRITE | AV_PERM_PRESERVE |
> AV_PERM_REUSE2,
> link->w, link->h);
> + AVFILTER_GET_BUFREF_VIDEO_PROPS(pic_props, picref);
>
> av_picture_copy((AVPicture *)&picref->data, (AVPicture *)&c->frame,
> picref->format, link->w, link->h);
>
> - picref->pts = c->pts;
> - picref->pixel_aspect = c->pixel_aspect;
> - picref->interlaced = c->frame.interlaced_frame;
> - picref->top_field_first = c->frame.top_field_first;
> + picref->pts = c->pts;
> + pic_props->pixel_aspect = c->pixel_aspect;
> + pic_props->interlaced = c->frame.interlaced_frame;
> + pic_props->top_field_first = c->frame.top_field_first;
> avfilter_start_frame(link, avfilter_ref_buffer(picref, ~0));
> avfilter_draw_slice(link, 0, link->h, 1);
> avfilter_end_frame(link);
Looks fine otherwise, regards.
--
FFmpeg = Friendly & Faithless Miracolous Patchable Erratic God
More information about the ffmpeg-devel
mailing list