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.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
