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 1D99142BC6 for ; Thu, 1 Sep 2022 13:01:38 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id BD1CA68B7A1; Thu, 1 Sep 2022 16:01:35 +0300 (EEST) Received: from nef.ens.fr (nef2.ens.fr [129.199.96.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id B6A6968B7A1 for ; Thu, 1 Sep 2022 16:01:28 +0300 (EEST) 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 281D1RhA028877 for ; Thu, 1 Sep 2022 15:01:28 +0200 Received: by phare.normalesup.org (Postfix, from userid 1001) id C605EE0101; Thu, 1 Sep 2022 15:01:27 +0200 (CEST) Date: Thu, 1 Sep 2022 15:01:27 +0200 From: Nicolas George To: FFmpeg development discussions and patches Message-ID: References: <20220824151828.24218-1-george@nsup.org> <20220831230358.GA3557@mariano> MIME-Version: 1.0 In-Reply-To: <20220831230358.GA3557@mariano> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (nef.ens.fr [129.199.96.32]); Thu, 01 Sep 2022 15:01:28 +0200 (CEST) Subject: Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter 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="===============6273134555614421023==" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: --===============6273134555614421023== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="C3RbuZgSFqNWW5ZY" Content-Disposition: inline --C3RbuZgSFqNWW5ZY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Stefano Sabatini (12022-09-01): > serializeS Locally fixed. > This "something" is vague and can be confused with the "something" in > "av_something_write", maybe something as: > when presented with invalid arguments, if the flag is not set the > called method should leave... No, the first interpretation was exactly what it meant, it is the same "something". For exemple, with "something" =3D "sample format", av_get_sample_fmt_name(42) returns NULL, av_sample_fmt_write(wr, 42, 0) writes nothing but av_sample_fmt_write(wr, 42, AV_WRITER_FALLBACK) writes "invalid". > what variable? =3D> in the AVWriter data? Locally changed to: + * The buffer is initially kept in the AVWriter data itself, it switches to + * heap allocation if it grows too large. In that case, + * av_writer_get_error() can return AVERROR(ENOMEM) if the allocation fail= s. > > +char *av_dynbuf_writer_get_buffer(AVWriter wr, size_t size, size_t *rs= ize); > > + * size must be <=3D *rsize on a previous call to > > + * av_dynbuf_writer_get_buffer(). > what is *rsize? One of the arguments to av_dynbuf_writer_get_buffer(), explained just above. > is called =3D> if called Fixed. > > +#define av_buf_writer_array(array) \ > > + av_buf_writer(array, sizeof (array)) > av_buf_writer_from_array? I would rather keep the name concise, but I will change if other agree your version is better or if you insist. > > +AVWriter av_stdio_writer(FILE *out); > av_file_writer() ? That would be ambiguous, it would suggest writing to an actual file. > I'd use a verb for consistency/clarify. > notify_impossibility or notify_impossible_op? Locally changed to: - void (*impossible)(AVWriter wr, const char *message); + void (*notify_impossible)(AVWriter wr, const char *message); > > + * If *size is returned < min_size, data may get discarded. > If the returned *size is < min_size... ? Locally changed to: - * If *size is returned < min_size, data may get discarded. + * If the returned *size is < min_size, data may get discarded. > > + * size is guaranteed <=3D the size value returned by get_buffer(). > size is or should be guaranteed? Is. The documentation is written for somebody who implements the methods, and it tells them they do not have to bother checking size here (although an assert would not be bad in this kind of situation). I added a paragraph in the introduction: * Applications that want to implement other kinds of AVWriter * need to create a structure with some of these methods implemented. * + * These methods should only be called directly by the implementation of + * AVWriter. Their documentation is written from the point of view of + * implementing them. + * * Every type of AVWriter is supposed to have a pair of functions: * av_something_writer_get_methods() returns the methods for that type. * av_something_writer_checks() returns 1 if the writer is of that type. Thanks for the review. I do not think it is necessary to send an updated patch for these changes, is it? Regards, --=20 Nicolas George --C3RbuZgSFqNWW5ZY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE6ooRQGBoNzw0KnwPcZVLI8pNxgwFAmMQrSUACgkQcZVLI8pN xgyCSA//RU//rJWwoPcRIVqgpHFoUuYB++b0caz6jK8/FlKTMooo2+NW/LH6fq8Y 6GNcFrQ68SUlcmWPn1rA1B+QJuBO3CiFThe/j8XNC8DDUYKGdzqwrK0MAUH7zd8j y2GKi10GQrkt1TFDO+l9HLm9ldmqyjqTuaHNYjcMpqmdtXISiuXIZMroAtdi5YZl IBFn2H40Ry+QLw3g5SHEMhz82ehpGyktXgQ0DGnZeHiY4jFQC0vhJ4x2tNc6NsAj hKK/7awBKicfFxp7u5qFOE/iMiD1VqcBLOazN8IoYUlsCGfGWpDWfrv8nMJhkc14 mK7VUM85gqZpE3Qbz/AuCN3Q/X2PUaAxzOLHthSulB6gS1P7dYrYHvsyhhINPu6C bKST60ilJ+mfmqGlk9DwjiRoH9l9IqlrAKFT237KYzKcK8pqUawf3M1yNzBdRws3 SlQfPm4q2+pb6ZrjtViW9GyDgUoiUfmVvIVxv4Ct4coM/vKfYrwMRmS+XSNMqEnD nq8UnQNq8KBqsCcdtdywG3YKFittMS+mxDOu9RT5n85am4/r9ur5o+L21Hob/kpX Ni1WipN31G802VDxN9OHEGDYicKW7/tKTMUA9TC9ChjSoOTeIC/yoBAWL2H5z8zv Yo5Hof7pun/rzVLRKHM9bZAPqg4493gFhvv4AaPukm708Kg4cWo= =MqO6 -----END PGP SIGNATURE----- --C3RbuZgSFqNWW5ZY-- --===============6273134555614421023== 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". --===============6273134555614421023==--