On Mon, Mar 13, 2023 at 3:30 PM Marton Balint <c...@passwd.hu> wrote: > > > > On Fri, 10 Mar 2023, Marton Balint wrote: > > > > > > > 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. > > Oh, this patch needs a fate update as well. On that note, DNXHD is a > macroblock-based codec, so why are we changing 1088 height to 1080? I > could ask the same for ProRes. The patch should explain better why those > need to be changed, if they do.
FYI. The relationship between stored height/width and the corresponding Prores parameters (horizontal_size, vertical_size and bottomFieldVerticalSize) is specified in SMPTE RDD 44. > > 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". _______________________________________________ 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".
