From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id E32CC4746E for ; Tue, 10 Oct 2023 11:29:06 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8647968CB43; Tue, 10 Oct 2023 14:29:04 +0300 (EEST) Received: from mail0.khirnov.net (red.khirnov.net [176.97.15.12]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id EABBB68C3D6 for ; Tue, 10 Oct 2023 14:28:57 +0300 (EEST) Received: from localhost (localhost [IPv6:::1]) by mail0.khirnov.net (Postfix) with ESMTP id AF63024048D for ; Tue, 10 Oct 2023 13:28:57 +0200 (CEST) Received: from mail0.khirnov.net ([IPv6:::1]) by localhost (mail0.khirnov.net [IPv6:::1]) (amavis, port 10024) with ESMTP id KYsTBRpVQChX for ; Tue, 10 Oct 2023 13:28:57 +0200 (CEST) Received: from lain.khirnov.net (lain.khirnov.net [IPv6:2001:67c:1138:4306::3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "lain.khirnov.net", Issuer "smtp.khirnov.net SMTP CA" (verified OK)) by mail0.khirnov.net (Postfix) with ESMTPS id 11DCF2400FF for ; Tue, 10 Oct 2023 13:28:57 +0200 (CEST) Received: by lain.khirnov.net (Postfix, from userid 1000) id E5B381601B9; Tue, 10 Oct 2023 13:28:56 +0200 (CEST) From: Anton Khirnov To: FFmpeg development discussions and patches In-Reply-To: References: <20231007162503.1057-1-jamrial@gmail.com> <20231007162503.1057-5-jamrial@gmail.com> <169693619274.6638.10416615280449969394@lain.khirnov.net> <169693658102.6638.10089742736405247188@lain.khirnov.net> Mail-Followup-To: FFmpeg development discussions and patches Date: Tue, 10 Oct 2023 13:28:56 +0200 Message-ID: <169693733685.6638.10997736231850859120@lain.khirnov.net> User-Agent: alot/0.8.1 MIME-Version: 1.0 Subject: Re: [FFmpeg-devel] [PATCH 4/7] avformat/avformat: add a flag to signal muxers that support storing cropping values X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: Quoting James Almer (2023-10-10 13:21:42) > 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 > >>>> --- > >>>> 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. As I said above - there are MANY similar bits of information that the muxer may or may not write. You cannot handle all of them with this approach, and singling out cropping seems horribly ad-hoc to me. -- Anton Khirnov _______________________________________________ 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".