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] [PATCH v2 5/5] fftools/ffmpeg: support applying container level cropping
Date: Mon, 31 Jul 2023 17:53:55 +0200
Message-ID: <db8e78935ff25133cbc11cb92fbc51b851b5745c.camel@haerdin.se> (raw)
In-Reply-To: <fc363ddb-b58f-24aa-9323-210b18949626@gmail.com>

tor 2023-07-27 klockan 08:59 -0300 skrev James Almer:
> On 7/27/2023 8:13 AM, Anton Khirnov wrote:
> > Quoting Tomas Härdin (2023-07-26)
> > > tis 2023-07-25 klockan 14:09 -0300 skrev James Almer:
> > > > Signed-off-by: James Almer <jamrial@gmail.com>
> > > > ---
> > > > Now inserting a filter into the graph.
> > > 
> > > This looks useful for MXF
> > > 
> > > > +    { "apply_cropping",   HAS_ARG | OPT_BOOL | OPT_SPEC |
> > > > +                          OPT_EXPERT |
> > > > OPT_INPUT,                                { .off =
> > > > OFFSET(apply_cropping) },
> > > > +        "Apply frame cropping instead of exporting it" },
> > > 
> > > Hm. Can this be applied automatically for ffplay? When
> > > transcoding I
> > > expect the typical use case is to not crop and to carry the
> > > metadata
> > > over.
> > 
> > Why? We do apply decoder cropping by default. There's also no
> > guarantee
> > that your container will be able to write it, so it seems better to
> > apply it by default.

Not necessarily. Doing this by default may break some downstream
projects. The relevant metadata must be deleted if this is done, so
that the cropping isn't done twice when you get to playout.

> I agree. In a transcoding scenario you want to apply the container
> level 
> cropping since it's defining a subrectangle with the actual content 
> meant for display, so why force the encoder handle pixels that were 
> meant to be discarded to being with, potentially ruining encoding 
> quality for neighboring pixels?
> 
> For codec copy scenarios though, the side data is going to be copied,
> so 
> Tomas' idea of having muxers report they support writing it is good 
> either way.

My main concern is not losing pixels if we can avoid it, even if those
pixels are invisible. On the other hand, when transcoding, we could go
with always cropping unless the user requests otherwise. This has the
benefit of essence dimensions not changing with container. Also less
work for the encoder. But again, this is a behavior change that may
break things downstream.

Basically what I'm suggesting is that ffplay behave as playout. We
could have ffmpeg behave similarly but we should keep in mind this may
break some workflows.

/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-07-31 15:54 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-19 22:20 [FFmpeg-devel] [PATCH 1/5] avcodec/packet: add a decoded frame cropping side data type James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 2/5] avformat/dump: print Frame Cropping side data info James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 3/5] avformat/matroskadec: export cropping values James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 4/5] avformat/matroskaenc: support writing " James Almer
2023-07-19 22:20 ` [FFmpeg-devel] [PATCH 5/5] fftools/ffmpeg: support applying container level cropping James Almer
2023-07-20 19:08   ` Anton Khirnov
2023-07-20 19:25     ` James Almer
2023-07-20 19:31       ` Anton Khirnov
2023-07-20 19:39         ` James Almer
2023-07-25 17:09 ` [FFmpeg-devel] [PATCH v2 " James Almer
2023-07-26 21:42   ` Tomas Härdin
2023-07-26 22:11     ` James Almer
2023-07-27 11:07       ` Tomas Härdin
2023-07-27 11:13     ` Anton Khirnov
2023-07-27 11:59       ` James Almer
2023-07-31 15:53         ` Tomas Härdin [this message]
     [not found]       ` <3EF017F1-A8DB-4934-A86C-8E17BC067DA0@cosmin.at>
2023-08-01 18:51         ` Cosmin Stejerean

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=db8e78935ff25133cbc11cb92fbc51b851b5745c.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