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

[Tomas Härdin]

On Mon, 2012-09-17 at 20:52 +0200, Reimar Döffinger wrote:
> On Mon, Sep 17, 2012 at 11:02:33AM +0200, Tomas Härdin wrote:
> > 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.
> 
> Uh, then codec->with and codec->height are set wrong,
> coded_width/coded_height are for storing the encoded dimensions.

So it shouldn't set codec->width and codec->height at all, but coded_*
instead? I'm confused..

I say it's wrong because you're not supposed to use the encoded
dimensions to compute SAR - you're supposed to use the dimensions of the
display rectangle, which is often smaller than the dimensions of the
essence.

> > (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.
> 
> I very much don't think that broken files should be a significant
> concern, at least not when it is at the expense of correct files.

Yes, I'm actually leaning towards this too. I don't think it's illegal
for an MXF file to not specify these dimensions at all though, in which
case you'd have to probe anyway.

For now a simple solution to this that should at least not mess anything
up is:

0. read the size display rectangle
1. if not present, let its size be that of the stored dimensions (S377m
says so BTW)
2. check that the size of the display rectangle is greater than zero and
less than or equal to the stored dimensions, and skip the next step with
a complaint if that isn't the case
3. compute SAR from DAR and display rectangle. remember to account for
interlaced FrameLayouts having half the height

/Tomas