Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: "Tomas Härdin" <git@haerdin.se>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] avformat/mxfenc: fix stored/sampled/displayed width/height
Date: Wed, 18 Jan 2023 11:10:10 +0100
Message-ID: <9441a12dafc5dfe444953014b43cf587cd18b022.camel@haerdin.se> (raw)
In-Reply-To: <1e94dee6-7418-1daa-f579-d8af025c0aa8@mediaarea.net>

mån 2023-01-16 klockan 15:28 +0100 skrev Jerome Martinez:
> On 16/01/2023 14:50, Tomas Härdin wrote:
> > lör 2023-01-14 klockan 16:48 +0100 skrev Jerome Martinez:
> > > 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)
> > Another option might be to omit these values for non-macroblock
> > codecs.
> 
> I planned to send a patch for removing all redundant information 
> (sampled width/height is stored width/height by default etc), but as
> it 
> is changing the current behavior of FFmpeg so can be more
> controversial 
> I preferred to have it in a different patch.

Such information may or may not be redundant depending on context.
Which is part of why MXF is such a headache. Outputting as correct
information as possible is probably the best approach.

> After this patch it would be only a bunch of "if (sampled!=stored)" 
> additions.
> Would it be fine to do that separately?

I don't know. The problem here is that we don't know what workflows
users have.  On the other hand if we're bold and remove redundant
information then we'll learn what users are actually relying on it.

> > There is also potentially one use-value: when remuxing BMP to MXF
> > it
> > may be necessary to deal with BMP's 16-bit alignment. I'm not sure
> > if
> > this happens in the wild, and we certainly don't support muxing it.
> 
> I don't get what I should change in the patch for that, AFAIK I don't
> touch stuff around that.

No need to do anything, it's just me noting when it might be the case
that stored != sampled for non-MB formats.

> > Another very close reading of the spec seems appropriate.
> 
> Tried to do so, as well as checking the other files I have from other
> muxers.
> When I don't get the difference between what is in specs and FFmpeg
> or 
> other muxer behavior, I let current FFmpeg behavior untouched.

Ugh, cargo culting. Not that I'm not guilty of the same..

/Tomas

_______________________________________________
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-01-18 10:10 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 [this message]
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
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=9441a12dafc5dfe444953014b43cf587cd18b022.camel@haerdin.se \
    --to=git@haerdin.se \
    --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