[FFmpeg-devel] [PATCH] mxfdec: export aspect information.

[Tomas Härdin]

On Sat, 2012-09-15 at 22:37 +0200, Reimar Döffinger wrote:
> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> ---
>  libavformat/mxfdec.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
> index e55c490..804975e 100644
> --- a/libavformat/mxfdec.c
> +++ b/libavformat/mxfdec.c
> @@ -1523,6 +1523,9 @@ static int mxf_parse_structural_metadata(MXFContext *mxf)
>                  default:
>                      av_log(mxf->fc, AV_LOG_INFO, "Unknown frame layout type: %d\n", descriptor->frame_layout);
>              }
> +            if (descriptor->aspect_ratio.num > 0 && descriptor->aspect_ratio.den > 0)
> +                st->sample_aspect_ratio = av_div_q(descriptor->aspect_ratio,
> +                    (AVRational){st->codec->width, st->codec->height});

This is wrong. st->codec->width/height are StoredWidth/Height. SAR
should be computed from the size of the display rectangle
(DisplayWidth/Height) if set. It's also possible for StoredWidth/Height
to be wrong (certain P2 files), in which case you must wait for the
dimensions to be probed before SAR is computed.

There are also files where the display resolution may be wrong - certain
files that pop out of Avid Media Composer have DisplayWidth =
SampledWidth * SAR. This can be solved by simply letting DisplayWidth =
SampledWidth.
DisplayWidth must be less than or equal to SampledWidth, same for
height. SampledWidth may be larger than StoredWidth IIRC, in which case
the frame should be padded with black. I haven't run into any such files
though, meaning I just pretend Sampled* don't exist (technically wrong,
but it works). See S377m Annex E.

Finally, there are files where neither rectangle is set.

At work I solved this by exposing DAR, DisplayWidth and DisplayHeight in
AVStream, then computing SAR in avformat_find_stream_info() after codec
dimensions have been probed. Like so:

     if (!st->r_frame_rate.num){
         if(    st->codec->time_base.den * (int64_t)st->time_base.num
             <= st->codec->time_base.num * st->codec->ticks_per_frame * (int64_t)st->time_base.den){
             st->r_frame_rate.num = st->codec->time_base.den;
             st->r_frame_rate.den = st->codec->time_base.num * st->codec->ticks_per_frame;
         }else{
             st->r_frame_rate.num = st->time_base.den;
             st->r_frame_rate.den = st->time_base.num;
         }
     }
+
+    /* if no display resolution set, take codec resolution */
+    if (!st->display_width.num)  st->display_width  = (AVRational){st->codec->width, 1};
+    if (!st->display_height.num) st->display_height = (AVRational){st->codec->height, 1};
+
+    /* if no container SAR, then compute it from container DAR and display resolution */
+    if (!st->sample_aspect_ratio.num && st->display_aspect_ratio.num) {
+        st->sample_aspect_ratio = av_div_q(st->display_aspect_ratio, av_div_q(st->display_width, st->display_height));
+    }
 }else if(st->codec->codec_type == AVMEDIA_TYPE_AUDIO) {

Oh yeah, you need to make sure IMX50 with VBI lines work with whatever
solution you come up with. So 720x608 with display rect = 720x576 @
(0,32). For DAR=4:3 SAR should pop out as 16:15, not whatever
4:3/720:608 works out to.

That's my core dump on this issue for now. For extra fun you could make
this work for MOV too, and when muxing both MXF and MOV.

/Tomas