Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Nicolas Gaullier <nicolas.gaullier@cji.paris>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] avformat/mxfenc: fix stored/sampled/displayed width/height
Date: Mon, 6 Mar 2023 17:09:43 +0000
Message-ID: <MR1P264MB24832027A32D77C62094C0999BB69@MR1P264MB2483.FRAP264.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <MR1P264MB2483FED3D78274E68A91B3469BC19@MR1P264MB2483.FRAP264.PROD.OUTLOOK.COM>

>>> 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

Nicolas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2023-03-06 17:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-14 15:48 Jerome Martinez
2023-01-14 20:04 ` Michael Niedermayer
2023-01-15 14:24   ` Jerome Martinez
2023-01-15 16:55     ` Michael Niedermayer
2023-01-16 13:50 ` Tomas Härdin
2023-01-16 14:28   ` Jerome Martinez
2023-01-18 10:10     ` Tomas Härdin
2023-01-16 14:00 ` Nicolas Gaullier
2023-01-16 15:49   ` Jerome Martinez
2023-01-16 18:15     ` Nicolas Gaullier
2023-03-06 17:09       ` Nicolas Gaullier [this message]
2023-03-10 21:10         ` Marton Balint
2023-03-13 22:30           ` Marton Balint
2023-03-13 22:39             ` Pierre-Anthony Lemieux
2023-03-14  9:41           ` Jerome Martinez
2023-03-26 19:32             ` Marton Balint

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=MR1P264MB24832027A32D77C62094C0999BB69@MR1P264MB2483.FRAP264.PROD.OUTLOOK.COM \
    --to=nicolas.gaullier@cji.paris \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git