On Mon, 6 Mar 2023, Nicolas Gaullier wrote:
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".
I totally agree they are so many weird things in the wild, particularly looking at
some early implementations. I also have fully broken DV100 and XDCAMHD35 Omneon
records with release v6.1 (2010) at the early stages of HD, but it was fixed
afterwards (with many other >issues too!). Looking at GVG, 1440x1088i stored
size was implemented from the early beginnings (2010 too) : sample clips are still
available here : http://www.gvgdevelopers.com/concrete/products/k2/test_clips/
There is also "kind of" reference sony implementation available here both for
xdcamhd35/avc-1440: https://www.sonycreativesoftware.com/catalystbrowse
Anyway, I think we all agree not to change anything related to MPEG2 and AVC.
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.
DV is questionable. Currently, the dust is under the rug (as a defaults
behaviour), which is an issue with very little concern.
Now, with the patch, the dust become visible, the DV rule is made explicit and
moreover it is presented as an exception, sharing code with macroblock
codecs... I think it is time to fix, even if it was not your prior intention.
I don't have an extensive experience with DV too, I just have samples here and
there like you, but it seems we share the same information.
Let see if someone else react and ask for keeping the current 1088 lines for DV
stored height, but if nobody does, I think it should be okay.
Do you want me to add a comment line e.g. "obey 'including any decoder scaling or
padding' from SMPTE ST 377"?
I am not a core developer and will let others give their feedback. My personal opinion is
that the spec is supposed to be well known and does not require commenting, but that it
would be interesting to explicit why we make a difference between DV and MPEG2/AVC. And
>personally, I don't have the answer to this question. If nobody has one, maybe a comment
could say "because this is the observed common practice".
Nicolas
Some weeks later now and no replies, maybe time to go on ?
I think the "case AV_CODEC_ID_DVVIDEO:" can be removed as discussed, fate
updated and that should be ok for everybody.
(Ideally, it could have been an opportunity to document why we have this "DV
exception", but I understand it is not very comfortable to write as there is no
meaningful reason, so forget about this, this won't hold up the patch anyway)
For information, there was a long thread recently on ffmpeg-user about a "bug"
in dnxhd stored_height (will be fixed with your patch):
https://ffmpeg.org/pipermail/ffmpeg-user/2023-February/056111.html
Will apply the patch in a couple of days unless somebody objects. If you
want to change DV height (seems reasonable), please send a follow up
patch with fate updates after that.
Thanks,
Marton
_______________________________________________
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".
