[FFmpeg-devel] [PATCH 2/2] Add sample_aspect_ratio fields to vsrc_buffer arguments. This fixes aspect handling in ffmpeg This is based on a patch by baptiste

[Ronald S. Bultje]

Hi,

On Thu, Feb 3, 2011 at 7:16 PM, Stefano Sabatini
<stefano.sabatini-lala at poste.it> wrote:
> On date Thursday 2011-02-03 16:42:29 -0500, Mike Scheutzow encoded:
>> Mans Rullgard wrote:
>> >From: Michael Niedermayer <michaelni at gmx.at>
>> >--- a/ffmpeg.c
>> >+++ b/ffmpeg.c
>> >@@ -2845,6 +2855,10 @@ static void opt_frame_aspect_ratio(const char *arg)
>> > ? ? ? ? ffmpeg_exit(1);
>> > ? ? }
>> > ? ? frame_aspect_ratio = ar;
>> >+
>> >+ ? ?x = vfilters ? strlen(vfilters) : 0;
>> >+ ? ?vfilters = av_realloc(vfilters, x+100);
>> >+ ? ?snprintf(vfilters+x, x+100, "%csetdar=%f\n", x?',':' ', ar);
>> > }
>> > static int opt_metadata(const char *opt, const char *arg)
>>
>> 1. This will fail to compile if CONFIG_AVFILTER == 0
>>
>> 2. The user's attempt to set DAR may be silently ignored.
>>
>> This patch effectively rewrites "-aspect <float>" as "-vf
>> setdar=<float>". So the user's attempt to set DAR will be ignored
>> for a command line like this:
>>
>> ffmpeg -i infile -aspect 1.7778 -vf "scale=360:240" outfile
>>
>> This is because the current ffmpeg behavior is to keep only the
>> final -vf argument on the command line.
>>
>> Command line argument ordering is already very complicated in
>> ffmpeg. We shouldn't make it even more difficult to get the desired
>> output file.
>>
>> One solution is to have -vf abort (with a good error message) if
>> 'vfilters' is already non-null.
>
> Simpler variant of this, forces the user to use setdar if libavfilter
> is enabled.
[..]
> @@ -2825,6 +2835,11 @@ static void opt_frame_pix_fmt(const char *arg)
>
>  static void opt_frame_aspect_ratio(const char *arg)
>  {
> +#if CONFIG_AVFILTER
> +    av_log(NULL, AV_LOG_ERROR,
> +           "Option 'aspect' is incompatible with filters, use the setdar filter instead\n");
> +    ffmpeg_exit(1);
> +#else
>      int x = 0, y = 0;
>      double ar = 0;
>      const char *p;

Uhm, maybe a little more explicit (-vf "bla bla") would help the more
non-developer users. But in general I'm OK with this.

Ronald