[FFmpeg-devel] [RCF] lavfi aspect ratio setting path
Stefano Sabatini
stefano.sabatini-lala
Sun Nov 28 23:41:28 CET 2010
On date Wednesday 2010-11-24 00:50:17 -0800, Baptiste Coudurier encoded:
> Hi guys,
>
> On 11/20/10 8:01 AM, Michael Niedermayer wrote:
> > On Sat, Nov 20, 2010 at 12:42:52PM +0100, Stefano Sabatini wrote:
> >> On date Friday 2010-11-19 19:14:28 -0800, Baptiste Coudurier encoded:
[...]
> >>> The aspect filter handling is weird anyway, dump_format and thus
> >>> ffmpeg won't display the correct aspect ratio, I consider this
> >>> broken so I disable his effects.
> >>
> >> The aspect filters have another problem, they may fail with
> >> some encoder which needs to set the DAR/SAR when openining the
> >> context, because they set DAR/SAR frame by frame (while this *may* be
> >> a global property).
> >>
> >> If this is the case (I need confirmation from someone with more
> >> expertise in encoders), then the only way to fix the problem is to
> >> make the aspect a property of the link, and make it configurable
> >> during the configuration stage. In case the aspect changes during
> >> filtering, we may need to re-configure the filtergraph (as it should
> >> happen when w/h change).
> >
> > SAR should be at a similar level as w/h
> > alot of encoders and formats wont allow changing sar (like they dont allow w/h
> > changes)
> > and a sar change if it occurs is very likely also to change the w/h
> > and if we have a input that changes sar (and w/h) a scale filter somewhere
> > could be used to get rid of these changes for outputs that cant handle it
> >
>
> Great, here is a shoot at it.
>
> Btw, I disabled the par alteration in scale filter, to keep the current
> behaviour
> of ffmpeg.
> -aspect cli option works correctly.
>
> --
> Baptiste COUDURIER
> Key fingerprint 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
> FFmpeg maintainer http://www.ffmpeg.org
> diff --git a/ffmpeg.c b/ffmpeg.c
> index e58e7b5..c85c81b 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -351,6 +351,7 @@ static int configure_filters(AVInputStream *ist, AVOutputStream *ost)
> AVCodecContext *codec = ost->st->codec;
> AVCodecContext *icodec = ist->st->codec;
> FFSinkContext ffsink_ctx = { .pix_fmt = codec->pix_fmt };
> + AVRational pixel_aspect_ratio;
I suggest sample_aspect_ratio, for consistency with setsar and
AVCodecContext (AVFilterLink.pixel_aspect also should be renamed).
> char args[255];
> int ret;
>
> @@ -361,8 +362,18 @@ static int configure_filters(AVInputStream *ist, AVOutputStream *ost)
> if ((ret = avfilter_open(&ist->output_video_filter, &ffsink, "out")) < 0)
> return ret;
>
> - snprintf(args, 255, "%d:%d:%d:%d:%d", ist->st->codec->width,
> - ist->st->codec->height, ist->st->codec->pix_fmt, 1, AV_TIME_BASE);
> +
> + if (ist->st->sample_aspect_ratio.num)
> + pixel_aspect_ratio = ist->st->sample_aspect_ratio;
> + else if (ist->st->codec->sample_aspect_ratio.num)
> + pixel_aspect_ratio = ist->st->codec->sample_aspect_ratio;
> + else
> + pixel_aspect_ratio = (AVRational){1, 1};
> +
> + snprintf(args, 255, "%d:%d:%d:%d:%d:%d:%d", ist->st->codec->width,
> + ist->st->codec->height, ist->st->codec->pix_fmt, 1, AV_TIME_BASE,
> + pixel_aspect_ratio.num, pixel_aspect_ratio.den);
> +
> if ((ret = avfilter_init_filter(ist->input_video_filter, args, NULL)) < 0)
> return ret;
> if ((ret = avfilter_init_filter(ist->output_video_filter, NULL, &ffsink_ctx)) < 0)
> @@ -419,6 +430,10 @@ static int configure_filters(AVInputStream *ist, AVOutputStream *ost)
>
> codec->width = ist->output_video_filter->inputs[0]->w;
> codec->height = ist->output_video_filter->inputs[0]->h;
> + ost->st->sample_aspect_ratio = codec->sample_aspect_ratio =
> + frame_aspect_ratio == 0 ? // overriden by the -aspect cli option
> + av_d2q(frame_aspect_ratio*codec->height/codec->width, 255) :
> + ist->output_video_filter->inputs[0]->pixel_aspect;
>
> return 0;
> }
> @@ -1587,9 +1602,7 @@ static int output_packet(AVInputStream *ist, int ist_index,
> #if CONFIG_AVFILTER
> if (ist->st->codec->codec_type == AVMEDIA_TYPE_VIDEO && ist->input_video_filter) {
> // add it to be filtered
> - av_vsrc_buffer_add_frame(ist->input_video_filter, &picture,
> - ist->pts,
> - ist->st->codec->sample_aspect_ratio);
> + av_vsrc_buffer_add_frame(ist->input_video_filter, &picture, ist->pts);
> }
> #endif
>
> @@ -1646,10 +1659,6 @@ static int output_packet(AVInputStream *ist, int ist_index,
> do_audio_out(os, ost, ist, decoded_data_buf, decoded_data_size);
> break;
> case AVMEDIA_TYPE_VIDEO:
> -#if CONFIG_AVFILTER
> - if (ist->picref->video)
> - ost->st->codec->sample_aspect_ratio = ist->picref->video->pixel_aspect;
> -#endif
> do_video_out(os, ost, ist, &picture, &frame_size);
> if (vstats_filename && frame_size)
> do_video_stats(os, ost, frame_size);
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index a761c81..484b7f3 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -184,6 +184,9 @@ int avfilter_config_links(AVFilterContext *filter)
> link->time_base = link->src && link->src->input_count ?
> link->src->inputs[0]->time_base : AV_TIME_BASE_Q;
>
> + if (link->pixel_aspect.num == 0 && link->pixel_aspect.den == 0)
> + link->pixel_aspect = link->src->inputs[0]->pixel_aspect;
> +
> if ((config_link = link->dstpad->config_props))
> if ((ret = config_link(link)) < 0)
> return ret;
> diff --git a/libavfilter/avfilter.h b/libavfilter/avfilter.h
> index f49a523..e188ec1 100644
> --- a/libavfilter/avfilter.h
> +++ b/libavfilter/avfilter.h
> @@ -575,9 +575,10 @@ struct AVFilterLink {
>
> enum AVMediaType type; ///< filter media type
>
> - /* These two parameters apply only to video */
> + /* These three parameters apply only to video */
These parameters so no other updates are required.
> int w; ///< agreed upon image width
> int h; ///< agreed upon image height
> + AVRational pixel_aspect; ///< agreed upon pixel aspect
> /* These two parameters apply only to audio */
> int64_t channel_layout; ///< channel layout of current buffer (see libavcore/audioconvert.h)
> int64_t sample_rate; ///< samples per second
> diff --git a/libavfilter/defaults.c b/libavfilter/defaults.c
> index c7b4b47..ca5db23 100644
> --- a/libavfilter/defaults.c
> +++ b/libavfilter/defaults.c
> @@ -48,6 +48,7 @@ AVFilterBufferRef *avfilter_default_get_video_buffer(AVFilterLink *link, int per
> ref->video = av_mallocz(sizeof(AVFilterBufferRefVideoProps));
> ref->video->w = w;
> ref->video->h = h;
> + ref->video->pixel_aspect = link->pixel_aspect;
>
> /* make sure the buffer gets read permission or it's useless for output */
> ref->perms = perms | AV_PERM_READ;
> @@ -236,6 +237,7 @@ int avfilter_default_config_output_link(AVFilterLink *link)
> if (link->type == AVMEDIA_TYPE_VIDEO) {
> link->w = link->src->inputs[0]->w;
> link->h = link->src->inputs[0]->h;
> + link->pixel_aspect = link->src->inputs[0]->pixel_aspect;
> link->time_base = link->src->inputs[0]->time_base;
> } else if (link->type == AVMEDIA_TYPE_AUDIO) {
> link->channel_layout = link->src->inputs[0]->channel_layout;
> diff --git a/libavfilter/vf_aspect.c b/libavfilter/vf_aspect.c
> index 6f86bf2..1dcb937 100644
> --- a/libavfilter/vf_aspect.c
> +++ b/libavfilter/vf_aspect.c
> @@ -61,14 +61,6 @@ static av_cold int init(AVFilterContext *ctx, const char *args, void *opaque)
> return 0;
> }
>
> -static void start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
> -{
> - AspectContext *aspect = link->dst->priv;
> -
> - picref->video->pixel_aspect = aspect->aspect;
> - avfilter_start_frame(link->dst->outputs[0], picref);
> -}
> -
> #if CONFIG_SETDAR_FILTER
> /* for setdar filter, convert from frame aspect ratio to pixel aspect ratio */
> static int setdar_config_props(AVFilterLink *inlink)
> @@ -82,6 +74,9 @@ static int setdar_config_props(AVFilterLink *inlink)
>
> av_log(inlink->dst, AV_LOG_INFO, "w:%d h:%d -> dar:%d/%d par:%d/%d\n",
> inlink->w, inlink->h, dar.num, dar.den, aspect->aspect.num, aspect->aspect.den);
> +
> + inlink->pixel_aspect = aspect->aspect;
> +
> return 0;
> }
>
> @@ -97,7 +92,7 @@ AVFilter avfilter_vf_setdar = {
> .type = AVMEDIA_TYPE_VIDEO,
> .config_props = setdar_config_props,
> .get_video_buffer = avfilter_null_get_video_buffer,
> - .start_frame = start_frame,
> + .start_frame = avfilter_null_start_frame,
> .end_frame = avfilter_null_end_frame },
> { .name = NULL}},
>
> @@ -108,6 +103,16 @@ AVFilter avfilter_vf_setdar = {
> #endif /* CONFIG_SETDAR_FILTER */
>
> #if CONFIG_SETSAR_FILTER
> +/* for setdar filter, convert from frame aspect ratio to pixel aspect ratio */
> +static int setsar_config_props(AVFilterLink *inlink)
> +{
> + AspectContext *aspect = inlink->dst->priv;
> +
> + inlink->pixel_aspect = aspect->aspect;
> +
> + return 0;
> +}
> +
> AVFilter avfilter_vf_setsar = {
> .name = "setsar",
> .description = NULL_IF_CONFIG_SMALL("Set the pixel sample aspect ratio."),
> @@ -118,8 +123,9 @@ AVFilter avfilter_vf_setsar = {
>
> .inputs = (AVFilterPad[]) {{ .name = "default",
> .type = AVMEDIA_TYPE_VIDEO,
> + .config_props = setsar_config_props,
> .get_video_buffer = avfilter_null_get_video_buffer,
> - .start_frame = start_frame,
> + .start_frame = avfilter_null_start_frame,
> .end_frame = avfilter_null_end_frame },
> { .name = NULL}},
>
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index d99e0c1..63c5cdb 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -158,11 +158,6 @@ static void start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
>
> outlink->out_buf = outpicref;
>
> - av_reduce(&outpicref->video->pixel_aspect.num, &outpicref->video->pixel_aspect.den,
> - (int64_t)picref->video->pixel_aspect.num * outlink->h * link->w,
> - (int64_t)picref->video->pixel_aspect.den * outlink->w * link->h,
> - INT_MAX);
> -
> scale->slice_y = 0;
> avfilter_start_frame(outlink, avfilter_ref_buffer(outpicref, ~0));
> }
> diff --git a/libavfilter/vsrc_buffer.c b/libavfilter/vsrc_buffer.c
> index 74d9bf6..2da70d1 100644
> --- a/libavfilter/vsrc_buffer.c
> +++ b/libavfilter/vsrc_buffer.c
Missing docs update.
Rest of the patch looks good to me, thanks.
--
FFmpeg = Formidable & Fanciful Mind-dumbing Peaceless Emblematic Gangster
More information about the ffmpeg-devel
mailing list