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

[Jerome Martinez]

On 16/01/2023 15:00, Nicolas Gaullier wrote:
>> 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)
> The width is one thing; for whatever reason, there is a divergence between DV100 on one hand and AVCI/XDCAMHD35 on the other. In my understanding, in current practices, DV obey s337 (stored width includes scaling) but xdcam&avci does not, so current code is fine but maybe this is an opportunity to document this ?

AFAIK:
- DV in MXF: found old Omneon with no scaling for stored value, no 
sampled value (so stored value), scaled value for displayed value, old 
Quantel with scaling everywhere. From my understanding of spec, I would 
keep the scaling.
- MPEG-2 Video including XDCAMHD35 in MXF obey "including any decoder 
scaling or padding" wording with a 16x16 rounding for height, I have no 
file not 1920 or 3840 width
- AVC in MXF: found old Omneon or old Quantel  or old Telestream with no 
padding value for stored value (height of 540 for interlaced). I don't 
understand why it is not same as with MPEG-2 Video so I don't touch 
FFmpeg behavior there (rounding). Actually checking again SMPTE ST 
381-2013, there is an explicit example: "1088: 1080-line progressive".

Do you want me to add a comment line e.g. "obey 'including any decoder 
scaling or padding' from SMPTE ST 377"?

> The height is another topic, and in my information (checked against some samples from K2/Harmonic/bmx), DV height should not be rounded : maybe this patch is an opportunity to fix this ?

I don't have DV in MXF with non multiple of 16 (I thought that DV is 
only 720x576 or 720x480 or 1280x720 or 1920x1080, all values multiple of 
16) and don't know about video encoding in DV so I didn't want to change 
the behavior of FFmpeg when I don't know, but
case AV_CODEC_ID_DVVIDEO:
line could be definitely removed if it is fine for you.


>
> Note: please mind update fate (make fate-lavf-mxf_opatom GEN=1).

Checked (only the stored height changes, from 1088 to 1080 for this 
DNxHD file) and fate updated for next patch.