Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 4/7] avformat/avformat: add a flag to signal muxers that support storing cropping values
Date: Tue, 10 Oct 2023 08:21:42 -0300
Message-ID: <b1f65016-aa9b-431d-ac40-5e3008f5b060@gmail.com> (raw)
In-Reply-To: <169693658102.6638.10089742736405247188@lain.khirnov.net>

On 10/10/2023 8:16 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-10-10 13:13:46)
>> On 10/10/2023 8:09 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-10-07 18:25:00)
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    libavformat/avformat.h | 1 +
>>>>    libavformat/mux.c      | 8 ++++++++
>>>>    2 files changed, 9 insertions(+)
>>>>
>>>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>>>> index 9e7eca007e..c099ca8a01 100644
>>>> --- a/libavformat/avformat.h
>>>> +++ b/libavformat/avformat.h
>>>> @@ -500,6 +500,7 @@ typedef struct AVProbeData {
>>>>                                            The user or muxer can override this through
>>>>                                            AVFormatContext.avoid_negative_ts
>>>>                                            */
>>>> +#define AVFMT_CROPPING     0x80000 /**< Format supports storing cropping values */
>>>
>>> I have mixed feeelings about this patch, for a bunch of reasons:
>>> * It is quite ad-hoc - we don't do this for other side data types, and
>>>     this approach would not scale if we did.
>>> * If we do want to signal this, we probably want to distinguish between
>>>     support for global and per-packet values.
>>
>> This patch came to be after some discussion from the first iteration of
>> the set, where concerns about the cropping information being silently
>> lost if apply_cropping was disabled during a transcoding or codec copy
>> scenario where the output format didn't support storing said values.
>>
>>> * How do you expect this to be useful to the callers? I don't see this
>>>     flag actually being used in the ffmpeg CLI patch.
>>
>> It's a format flag. Muxers use it, and the generic mux.c code will print
>> a warning if needed.
> 
> Why is it public then?

So the library user can know beforehand if the cropping information will 
be lost or not, and choose accordingly. The warning is there for the 
cases where it was ignored.

I can add a check to the CLI for it, but other than to abort or outright 
ignore the user request to not apply cropping i don't see what it could 
do, as mux.c already prints a warning.
_______________________________________________
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-10-10 11:21 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-07 16:24 [FFmpeg-devel] [PATCH 0/7] Container level frame cropping parameters James Almer
2023-10-07 16:24 ` [FFmpeg-devel] [PATCH 1/7] avcodec/packet: add a decoded frame cropping side data type James Almer
2023-10-10 11:30   ` Andreas Rheinhardt
2023-10-07 16:24 ` [FFmpeg-devel] [PATCH 2/7] avformat/dump: print Frame Cropping side data info James Almer
2023-10-10 11:03   ` Anton Khirnov
2023-10-07 16:24 ` [FFmpeg-devel] [PATCH 3/7] avformat/matroskadec: export cropping values James Almer
2023-10-07 16:25 ` [FFmpeg-devel] [PATCH 4/7] avformat/avformat: add a flag to signal muxers that support storing " James Almer
2023-10-10 11:09   ` Anton Khirnov
2023-10-10 11:13     ` James Almer
2023-10-10 11:16       ` Anton Khirnov
2023-10-10 11:21         ` James Almer [this message]
2023-10-10 11:28           ` Anton Khirnov
2023-10-07 16:25 ` [FFmpeg-devel] [PATCH 5/7] avformat/matroskaenc: support writing " James Almer
2023-10-07 16:25 ` [FFmpeg-devel] [PATCH 6/7] fftools/ffmpeg: support applying container level cropping James Almer
2023-10-10 11:13   ` Anton Khirnov
2023-10-07 16:25 ` [FFmpeg-devel] [PATCH 7/7] fftools/ffplay: " James Almer
2024-01-17 18:39   ` Dmitrii Ovchinnikov
2023-10-10 11:17 ` [FFmpeg-devel] [PATCH 0/7] Container level frame cropping parameters Anton Khirnov
2023-10-10 11:44   ` James Almer
2023-10-10 11:54     ` Anton Khirnov
2023-10-10 11:58       ` James Almer
2023-10-10 12:12         ` Anton Khirnov

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=b1f65016-aa9b-431d-ac40-5e3008f5b060@gmail.com \
    --to=jamrial@gmail.com \
    --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