[FFmpeg-devel] avformat/mxfenc: fix stored/sampled/displayed width/height

[Tomas Härdin]

mån 2023-01-16 klockan 15:28 +0100 skrev Jerome Martinez:
> On 16/01/2023 14:50, Tomas Härdin wrote:
> > lör 2023-01-14 klockan 16:48 +0100 skrev Jerome Martinez:
> > > Before the patch:
> > > - stored values were rounded to upper 16 multiple also for
> > > formats
> > > not
> > > using macroblocks (should be st->codecpar->width and
> > > st->codecpar->height when not MPEG formats; note that I found no
> > > other
> > > muxer doing the rounding for AVC, only for MPEG-2 Video, but I
> > > find
> > > no
> > > reason in specs for doing the difference so I kept the rounding
> > > for
> > > AVC)
> > > - sampled and displayed widths were stored width (should be
> > > st->codecpar->width like it is already done for height, with the
> > > DV50/100 exception)
> > Another option might be to omit these values for non-macroblock
> > codecs.
> 
> I planned to send a patch for removing all redundant information 
> (sampled width/height is stored width/height by default etc), but as
> it 
> is changing the current behavior of FFmpeg so can be more
> controversial 
> I preferred to have it in a different patch.

Such information may or may not be redundant depending on context.
Which is part of why MXF is such a headache. Outputting as correct
information as possible is probably the best approach.

> After this patch it would be only a bunch of "if (sampled!=stored)" 
> additions.
> Would it be fine to do that separately?

I don't know. The problem here is that we don't know what workflows
users have.  On the other hand if we're bold and remove redundant
information then we'll learn what users are actually relying on it.

> > There is also potentially one use-value: when remuxing BMP to MXF
> > it
> > may be necessary to deal with BMP's 16-bit alignment. I'm not sure
> > if
> > this happens in the wild, and we certainly don't support muxing it.
> 
> I don't get what I should change in the patch for that, AFAIK I don't
> touch stuff around that.

No need to do anything, it's just me noting when it might be the case
that stored != sampled for non-MB formats.

> > Another very close reading of the spec seems appropriate.
> 
> Tried to do so, as well as checking the other files I have from other
> muxers.
> When I don't get the difference between what is in specs and FFmpeg
> or 
> other muxer behavior, I let current FFmpeg behavior untouched.

Ugh, cargo culting. Not that I'm not guilty of the same..

/Tomas