[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