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 0917942219 for ; Sun, 16 Jan 2022 11:27:42 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 390CB68AC87; Sun, 16 Jan 2022 13:27:40 +0200 (EET) Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 4BBB068A77A for ; Sun, 16 Jan 2022 13:27:33 +0200 (EET) X-ENS-nef-client: 129.199.129.80 ( name = phare.normalesup.org ) Received: from phare.normalesup.org (phare.normalesup.org [129.199.129.80]) by nef.ens.fr (8.14.4/1.01.28121999) with ESMTP id 20GBRW84007890 for ; Sun, 16 Jan 2022 12:27:32 +0100 Received: by phare.normalesup.org (Postfix, from userid 1001) id 5287EEB5BC; Sun, 16 Jan 2022 12:27:32 +0100 (CET) Date: Sun, 16 Jan 2022 12:27:32 +0100 From: Nicolas George To: FFmpeg development discussions and patches Message-ID: References: <20220113015101.4-1-jamrial@gmail.com> <20220113015101.4-2-jamrial@gmail.com> MIME-Version: 1.0 In-Reply-To: <20220113015101.4-2-jamrial@gmail.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Sun, 16 Jan 2022 12:27:32 +0100 (CET) Subject: Re: [FFmpeg-devel] [PATCH 001/281] Add a new channel layout API 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: multipart/mixed; boundary="===============5100838711236623592==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============5100838711236623592== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="BfTB7Pi43YvsbFrU" Content-Disposition: inline --BfTB7Pi43YvsbFrU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable James Almer (12022-01-12): > From: Anton Khirnov >=20 > The new API is more extensible and allows for custom layouts. > More accurate information is exported, eg for decoders that do not > set a channel layout, lavc will not make one up for them. >=20 > Deprecate the old API working with just uint64_t bitmasks. >=20 > Expanded and completed by Vittorio Giovara > and James Almer . > Signed-off-by: Vittorio Giovara > Signed-off-by: James Almer > --- > libavutil/channel_layout.c | 629 ++++++++++++++++++++++++++++++++----- > libavutil/channel_layout.h | 542 ++++++++++++++++++++++++++++++-- > libavutil/version.h | 1 + > 3 files changed, 1069 insertions(+), 103 deletions(-) Thank you. I have no fundamental objection to the design of the API as it is, but the user interface and documentation is still missing, that needs to be addressed before the patch goes in. (But IIRC, Marton had other requirements, so let us wait for him to weigh in.) > +/** > + * Initialize a channel layout from a given string description. > + * The input string can be represented by: > + * - the formal channel layout name (returned by av_channel_layout_desc= ribe()) > + * - single or multiple channel names (returned by av_channel_name() > + * or concatenated with "+") > + * - a hexadecimal value of a channel layout (eg. "0x4") > + * - the number of channels with default layout (eg. "5c") > + * - the number of unordered channels (eg. "4", "4C", or "4 channels") > + * > + * @param channel_layout input channel layout > + * @param str string describing the channel layout > + * @return 0 channel layout was detected, AVERROR_INVALIDATATA otherwise > + */ > +int av_channel_layout_from_string(AVChannelLayout *channel_layout, > + const char *str); The documentation for the syntax needs to be in the user documentation, with examples, not just in the API documentation. > +/** > + * This is the inverse function of @ref av_channel_name(). > + * > + * @return the channel with the given name > + * AV_CHAN_NONE when name does not identify a known channel > + */ > +enum AVChannel av_channel_from_string(const char *name); > +/** > + * Get a channel described by the given string. > + * > + * This function accepts channel names in the same format as > + * @ref av_channel_from_string(). > + * > + * @param channel_layout input channel layout > + * @return a channel described by the given string, or a negative AVERRO= R value. > + */ > +int av_channel_layout_channel_from_string(const AVChannelLayout *channel= _layout, > + const char *name); This looks to be the preferred function for when the user will specify a channel in a layout. First, this fact should be stated clearly in the introduction of the documentation. Otherwise, people will likely use other functions, probably av_channel_layout_channel_from_index(). Second, the "name" argument cannot be just a name argument: the user must be able to say "the third FC channel" or "the FC channel with name 'piano'". And probably both at once. idx =3D av_channel_layout_channel_from_string(layout, "FC"); idx =3D av_channel_layout_channel_from_string(layout, "FC#2"); idx =3D av_channel_layout_channel_from_string(layout, "FC[piano]"); idx =3D av_channel_layout_channel_from_string(layout, "FC[piano]#2"); (I think it would be acceptable to limit the name, for example "names with non-alphanumeric ASCII characters are not supported.) And this need to go in the user documentation. I am not sure if we also need a function to extract "all the FL channels with name 'piano'". Regards, --=20 Nicolas George --BfTB7Pi43YvsbFrU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE6ooRQGBoNzw0KnwPcZVLI8pNxgwFAmHkASIACgkQcZVLI8pN xgzpAw//Qrh60K/T93/eW+tJlWEmi+q05eT2LohNbF+2ipC8yfa+lU/lhhIcwlg+ VEsJG4K5UEGeFAaq4H31mntdmScUoLmaILfRAuseY348xi0VQR2SkywMoQX6aAGx PYDY+8jOsuJHSn8sWOZ3VOg2Kx9dnrWCqCk244hxlTv/bDFSJboax/o1nR3Fv2o5 MFZN/7M19GgWR/h8ZBfgcyNEEZZ87ie9ZuO2yN7YrhgE1XT9jjwJvtViqCTf0Uzl YfPODN8Qwu7BHwQUJD/ajYe6s2Nk5rHM9Klx+nYMi23fNOktf0TrPWX5tSD7AgVP wgSoIvDditoDoS9UBmWYP8Y7+XXSyK/OoQcDnm0EBS0b7rUnWjObiwQG4biWc5ju n1Fnpi3UxybrKQFXW75UnZFo4EOHRIhPArJYfeKvKL4Iz5CnmsuMhrRxcf+DtM5V Oab6rI/BXH0hUAom1Pn5nnRwuNBi2PZBbxYiF3k5Fmd/M99ZG9CAPTVxfSTsKW56 hl9We1xIl52k6y5d31oteNpPope1wsqb230FRieSyqkzjdPiJLuxfppe3FvPN7LM S4wExZukSn8b6ZJ8uQhEkmbMUC6EyMbEec/o2RDjMfCT9cMkpNW6ueSBMXs9EzGE dQDpqf/qhi8eGa0zXD+W1hITZYubBr1TPRhxaGUpp3EfIN8RRIU= =XniX -----END PGP SIGNATURE----- --BfTB7Pi43YvsbFrU-- --===============5100838711236623592== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ 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". --===============5100838711236623592==--