[FFmpeg-devel] [PATCH/RFC] honor DAR aspect ratio in nuv files.

[Måns Rullgård]

elupus <elupus at ecce.se> writes:

> Hi, 
>
> Apperently, the nuv format doesn't store PAR in it's header
> nowadays. It's actually DAR. So the following patch should handle
> that case (pasted inlined, will resend attached later after some
> other comments). This is how the myth decoder handles the old types
> of files.
>
> However, from what I can tell from the myth nuv recorder's history,
> the aspect was never really a valid aspect before, just hardcoded to
> 1.0 all the time. Thus maybe it's better to not set
> sample_aspect_ratio when the file header indicates an aspect of 1.0,
> as the video codec would probabably be a better source for this
> information then.
>
> Also, the nuv format allows for the aspect to change mid stream, by
> resending the file header. I'm not sure this is something that is
> allowed in ffmpeg, as it wouldn't be caught if you happended to seek
> past that new fileheader. Any thought on how to handle this? (i'm
> quite happy to just ignore it)
>
> Index: libavformat/nuv.c
> ===================================================================
> --- libavformat/nuv.c	(revision 13656)
> +++ libavformat/nuv.c	(working copy)
> @@ -137,6 +137,8 @@
>      get_byte(pb); // 'P' == progressive, 'I' == interlaced
>      url_fskip(pb, 3); // padding
>      aspect = av_int2dbl(get_le64(pb));
> +    if(aspect < 0.9999 && aspect > 1.0001)

That condition can never be true.

> +        aspect = 4.0 / 3.0;
>      fps = av_int2dbl(get_le64(pb));
>
>      // number of packets per stream type, -1 means unknown, e.g. streaming
> @@ -156,7 +158,10 @@
>          vst->codec->width = width;
>          vst->codec->height = height;
>          vst->codec->bits_per_sample = 10;
> -        vst->codec->sample_aspect_ratio = av_d2q(aspect, 10000);
> +        av_reduce(&vst->codec->sample_aspect_ratio.num,
> +                  &vst->codec->sample_aspect_ratio.den,
> +                  height * aspect
> +                  width, 255);
>          vst->r_frame_rate = av_d2q(fps, 60000);
>          av_set_pts_info(vst, 32, 1, 1000);
>      } else

Was the old code valid for some files?  If yes, this change is wrong
for those files.

-- 
M?ns Rullg?rd
mans at mansr.com