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 8F94D49BE7 for ; Wed, 3 Apr 2024 18:55:26 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E767268CFE3; Wed, 3 Apr 2024 21:55:23 +0300 (EEST) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 381A568C90C for ; Wed, 3 Apr 2024 21:55:17 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1712170516; bh=5u7cmkKSlMUvgUs1lHj294xucgBH795YqAuwBjNihJk=; h=Date:From:To:Subject:In-Reply-To:References:From; b=FuOQbNVzmuE133e0SEQzS0EhQag/lBvvB/RjE+gi7ajWtG3x3rH9HLHpA2CPeihJK S3SGa2lFOqwjwubtL/CPT4pJYnX5r/E8Jbe/FPuS8eAjcKhxWRNf+QxtvhlLUdLukz KDVnjDOrmD6j6B/HM0yo16ZdsdxZ1eBQXYHL9AN8= Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id E23AE4105D for ; Wed, 3 Apr 2024 20:55:16 +0200 (CEST) Date: Wed, 3 Apr 2024 20:55:16 +0200 Message-ID: <20240403205516.GB15225@haasn.xyz> From: Niklas Haas To: ffmpeg-devel@ffmpeg.org In-Reply-To: References: <20240205174413.92730-1-ffmpeg@haasn.xyz> <20240209131138.GB12126@haasn.xyz> MIME-Version: 1.0 Content-Disposition: inline Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec: add YUV color space metadata to AVCodec 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: On Sun, 24 Mar 2024 14:04:37 +0100 Andreas Rheinhardt wrote: > Niklas Haas: > > On Mon, 05 Feb 2024 19:04:30 +0100 Andreas Rheinhardt wrote: > >> This presumes the relevant states to be a cartesian product. Which need > >> not be true. A callback would be better; this would also allow to base > >> the list on other values already set in an AVCodecContext. And if this > >> were extended, it would also allow to remove init_static_data one day. > >> It is furthermore quite wasteful to store color_ranges in a list, > >> although there are only very few states for it. > > > > There is also the consideration to be made that using a callback is > > inconsistent with the established design. Consider that framerates, > > pix_fmts, samplerates, sample fmts and channel layouts are all currently > > provided as static arrays in AVCodec. There is a natural symmetry > > between these items and the ones I intend to add (yuv matrix, range, > > chroma location, primaries and gamma) - all of them are descriptive of > > the way data is encoded, and are therefore also (or should be) > > negotiable filter link properties. > > > > If we add a new callback API, should we then extend it to also include > > all of the existing items from the above list? Is there a reason that > > yuv range supports needs to be more dynamic than the others? > > > > It should support everything; and I'd like to remove the other (public) > static lists, too (after the necessary deprecations). > > > Food for thought: mjpeg is not the only codec that puts restrictions on > > the format support based on the strictness level. For example, > > yuv4mpegpipe_muxer errors out with a strictness warning if you use > > a non-standard pixel format. And arguably, in this case, this is > > **preferred** behavior over "silently" inserting a scale filter to > > convert to a supported format, as the whole point of y4m2 is to > > encapsulate raw data as-is. > > > > Should we: > > > > 1. Add a new dynamic callback that can query lists for all of the above > > in a way dependent on the strictness level, and use it as > > a replacement for the static lists currently in AVCodec? > > > > 2. Continue with the status quo of having these lists be static, plus > > dynamic checks at open() time, and continue using the "convenience > > hack" of having ffmpeg_tools automatically restrict limited range mjpeg? > > > > I really want this convenience hack removed. > > > It is not immediately obvious to me that an automatic conversion to > > a supported format is *necessarily* preferred to erroring out unless the > > user specifies a lower strictness level. > > I agree. (In fact, on default strictness, the current code inserts a > scale filter even if one explicitly adds "-color_range tv".) > > > > > As for an API, I think that rather than having an AVCodecContext-aware > > callback at all, I would just make callbacks that directly ingest the > > strictness level in AVCodec. That makes it far less of a black box about > > which fields of the AVCodecContext are relevant here. > > > > i.e. > > > > struct AVCodec { > > const enum AVColorRange (*get_color_ranges)(int strictness); > > const enum AVColorSpace (*get_color_spaces)(int strictness); > > // ditto for the other parameters? > > } > > Your callbacks would hardcode that the only thing that matters is > strictness. And it would be very expensive, because these fields would > be in every AVCodec, even though only a minority of AVCodecs (namely > video encoders) would use them. (supported_framerates is even only set > by two encoders. What a waste.) Adding an API like > > int avcodec_get_supported_config(const AVCodecContext *avctx, const > AVCodec *codec, void **supported_configs, unsigned *num_configs, enum > AVCodecConfigs desired_config, > unsigned flags, void *logctx); > (enum AVCodecConfigs would contain a value for pix fmts, sample fmts etc.) Having an extra `logctx` here seems redundant and inconsistent with other avctx-taking functions, which log to the provided `avctx`. > > allows to keep the details hidden and therefore use a compact way to > store it. > > - Andreas > > _______________________________________________ > 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". _______________________________________________ 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".