Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
James Almer (12023-04-25): > I support the addition of a new better API as a replacement for AVBprint, as > long as you take into account the suggestions made by people and their > requests for exemplifications of its usage and usefulness. This also means > potential changes to the header. Of course. If you notice remarks that I neglected to take into consideration, please remind me of them. > And please, do not ignore or dismiss any of them. No one is out to block > your work for the sake of it. By the way, thanks for intervening on my behalf on Monday. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Anton Khirnov (12023-04-26): > This project is developed in the open, private emails, rumors, and > hearsay do not count as arguments. > > This code is unnecessary bloat that adds nothing of value to the > project. Your negative opinion has already been taken into consideration, it is the main reason I write “significant more support than opposition” and not “no opposition at all”. -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Quoting Nicolas George (2023-04-25 19:11:44) > Hi. > > I finally have some tome to go at this again. > > Totalizing this thread, the previous discussions and the few private > mails I received, I conclude there is significant more support than > opposition to AVWriter, so I am moving forward with polishing and > pushing the implementation. This project is developed in the open, private emails, rumors, and hearsay do not count as arguments. This code is unnecessary bloat that adds nothing of value to the project. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On 4/25/2023 2:11 PM, Nicolas George wrote: Hi. I finally have some tome to go at this again. Totalizing this thread, the previous discussions and the few private mails I received, I conclude there is significant more support than opposition to AVWriter, so I am moving forward with polishing and pushing the implementation. (To avoid antagonizing people, and because I knew a new job would mean little time to work on it soon, I refrained from pushing the header, but consider it done anyway: the fact that the implementation of AVWriter is not present in FFmpeg is now a bug that needs to be fixed.) Regards, I support the addition of a new better API as a replacement for AVBprint, as long as you take into account the suggestions made by people and their requests for exemplifications of its usage and usefulness. This also means potential changes to the header. And please, do not ignore or dismiss any of them. No one is out to block your work for the sake of it. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Hi. I finally have some tome to go at this again. Totalizing this thread, the previous discussions and the few private mails I received, I conclude there is significant more support than opposition to AVWriter, so I am moving forward with polishing and pushing the implementation. (To avoid antagonizing people, and because I knew a new job would mean little time to work on it soon, I refrained from pushing the header, but consider it done anyway: the fact that the implementation of AVWriter is not present in FFmpeg is now a bug that needs to be fixed.) Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On Wed, Sep 07, 2022 at 03:30:09PM +0200, Nicolas George wrote: > Anton Khirnov (12022-09-02): > > As I already said to you in private, I do not think the motivation and > > use cases for this have been sufficiently established. > > > > You claim this will bring massive advantages all over the place. You > > should support these claims with some actual patches that demonstrate > > these advantages on some real code. > > I have not claimed “massive advantages all over the place”, I have > claimed minor advantages all over the places and massive advantages for > future API. > > With that correction stated, I have already answered you that your > inability to see the benefits is only proof that your work on FFmpeg is > on areas that rarely need strings. In fact, if I remember correctly, the > last time it happened you used the worst possible implementation and we > had to be multiple persons insisting to get it fixed. > > Therefore, I consider your objection to be orders of magnitude less > important than the positive feedback I finally had got from people who > do use strings routinely, who in particular have used BPrint, are > familiar with the benefits it brought but also the limitations I want to > fix. > > If there are no more remarks in the next few days I will consider this > approved and start polishing the implementations. My oppinion about AVWriter was asked for, iam not sure my oppinion is worth that much but ok. 1. I think the escalating tone of the thread is bad for peoples blood pressure and as well for the eventual technical outcome as it pushed decissions and support in an emotional direction not a direction fouded in facts and solid choice. 2. If AVWriter was called bprint 2.0 you might have avoided this whole opposition. And it really is IMHO a next generation bprint API. 3. You are the author of bprint and this is a from my point of view a 2nd generation bprint. I see nothing controversal on this from a technical point of view. Its simply a step forward initiated by the original author of the old code I think we should move to this bprint 2.0 unless someone has a solid argument why bprint 2.0 is worse than bprint 1.0. In which case maybe that issue can then be fixed thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Nations do behave wisely once they have exhausted all other alternatives. -- Abba Eban signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Quoting Nicolas George (2022-09-07 15:30:09) > Anton Khirnov (12022-09-02): > > As I already said to you in private, I do not think the motivation and > > use cases for this have been sufficiently established. > > > > You claim this will bring massive advantages all over the place. You > > should support these claims with some actual patches that demonstrate > > these advantages on some real code. > > I have not claimed “massive advantages all over the place”, I have > claimed minor advantages all over the places and massive advantages for > future API. > > With that correction stated, I have already answered you that your > inability to see the benefits is only proof that your work on FFmpeg is > on areas that rarely need strings. In fact, if I remember correctly, the > last time it happened you used the worst possible implementation and we > had to be multiple persons insisting to get it fixed. > > Therefore, I consider your objection to be orders of magnitude less > important than the positive feedback I finally had got from people who > do use strings routinely, who in particular have used BPrint, are > familiar with the benefits it brought but also the limitations I want to > fix. If ad hominem is the best argument you have then this clearly needs a lot more work. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Jean-Baptiste Kempf (12022-09-07): > Open source communities are based on consensus. You cannot just discard an > opinion > as less important than other just because YOU consider it's not important. > > Especially about someone who is in the top 5 contributor of the project, in > the > last few years (with Michael, James, Andreas and Paul of course). I am not the one who has been ignoring the opinion of peers in this story. -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On Wed, 7 Sep 2022, at 15:30, Nicolas George wrote: > With that correction stated, I have already answered you that your > inability to see the benefits is only proof that your work on FFmpeg is [...] > last time it happened you used the worst possible implementation and we Please stop attacking people directly, like this. This is absolutely not acceptable. > Therefore, I consider your objection to be orders of magnitude less > important than the positive feedback I finally had got from people who Open source communities are based on consensus. You cannot just discard an opinion as less important than other just because YOU consider it's not important. Especially about someone who is in the top 5 contributor of the project, in the last few years (with Michael, James, Andreas and Paul of course). jb -- Jean-Baptiste Kempf - President +33 672 704 734 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Anton Khirnov (12022-09-02): > As I already said to you in private, I do not think the motivation and > use cases for this have been sufficiently established. > > You claim this will bring massive advantages all over the place. You > should support these claims with some actual patches that demonstrate > these advantages on some real code. I have not claimed “massive advantages all over the place”, I have claimed minor advantages all over the places and massive advantages for future API. With that correction stated, I have already answered you that your inability to see the benefits is only proof that your work on FFmpeg is on areas that rarely need strings. In fact, if I remember correctly, the last time it happened you used the worst possible implementation and we had to be multiple persons insisting to get it fixed. Therefore, I consider your objection to be orders of magnitude less important than the positive feedback I finally had got from people who do use strings routinely, who in particular have used BPrint, are familiar with the benefits it brought but also the limitations I want to fix. If there are no more remarks in the next few days I will consider this approved and start polishing the implementations. -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
As I already said to you in private, I do not think the motivation and use cases for this have been sufficiently established. You claim this will bring massive advantages all over the place. You should support these claims with some actual patches that demonstrate these advantages on some real code. -- 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On date Thursday 2022-09-01 15:01:27 +0200, Nicolas George wrote: [...] > Thanks for the review. I do not think it is necessary to send an updated > patch for these changes, is it? Not needed, thanks. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
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" = "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? => 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 fails. > > +char *av_dynbuf_writer_get_buffer(AVWriter wr, size_t size, size_t *rsize); > > + * size must be <= *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 => 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 <= 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, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On date Wednesday 2022-08-24 17:18:28 +0200, Nicolas George wrote:
> The actual implementation, tests and uses in the rest of
> FFmpeg code will be committed separately once the API is
> settled.
>
> Signed-off-by: Nicolas George
> ---
> doc/avwriter_intro.md | 109 ++
> libavutil/writer.h| 484 ++
> 2 files changed, 593 insertions(+)
> create mode 100644 doc/avwriter_intro.md
> create mode 100644 libavutil/writer.h
>
[...]
> diff --git a/libavutil/writer.h b/libavutil/writer.h
> new file mode 100644
> index 00..55e2cf3ea6
> --- /dev/null
> +++ b/libavutil/writer.h
> @@ -0,0 +1,484 @@
> +/*
> + * Copyright (c) 2022 The FFmpeg project
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#ifndef AVUTIL_WRITER_H
> +#define AVUTIL_WRITER_H
> +
> +#include
> +#include
> +#include
> +
> +#include "extendable.h"
> +#include "bprint.h"
> +
> +/**
> + * @defgroup av_writer AVWriter
> + *
> + * Object-oriented API to write strings and binary data.
> + *
> + * @{
> + */
> +
> +typedef struct AVWriterMethods AVWriterMethods;
> +
> +/**
> + * Opaque object to write strings and binary data.
> + *
> + * AVWriter is meant to allow to build and return strings (and blocks of
> + * binary data) efficiently between functions.
> + * For example, a function that serialize something into a string will
serializeS
> + * actually write into an AVWriter, and the caller will choose the way the
> + * data will be stored or processed on the fly.
> + *
> + * For a quick introduction on how to use AVWriter in simple cases, see
> + * doc/avwriter_intro.md.
> + *
> + * There are various pre-defined types of AVWriter, see below:
> + *
> + * - av_dynbuf_writer() writes the data into a buffer that is grown with
> + * av_realloc() if it does not fit in the initial buffer; it is the
> + * recommended choice.
> + *
> + * - av_buf_writer() writes the data into a pre-existing finite buffer.
> + *
> + * - av_stdio_writer() writes the data into a FILE*.
> + *
> + * - av_log_writer() writes the data to av_log().
> + *
> + * AVWriter objects are passed by value. It allows creating a writer on the
> + * fly, without extra variable or error checking. The structure can never
> + * change.
> + */
> +typedef struct AVWriter {
> +const AVWriterMethods *methods;
> +void *obj;
> +} AVWriter;
> +
> +/**
> + * Write a data buffer to an AVWriter.
> + */
> +void av_writer_write(AVWriter wr, const char *buf, size_t size);
> +
> +/**
> + * Write a 0-terminated string to an AVWriter.
> + */
> +void av_writer_print(AVWriter wr, const char *str);
> +
> +/**
> + * Write a formatted string to an AVWriter.
> + * Note: do not use libc-specific extensions to the format string.
> + */
> +void av_writer_printf(AVWriter wr, const char *fmt, ...);
> +
> +/**
> + * Write a formatted to string an AVWriter using a va_list.
> + */
> +void av_writer_vprintf(AVWriter wr, const char *fmt, va_list va);
> +
> +/**
> + * Write a sequence of identical chars to an AVWriter.
> + */
> +void av_writer_add_chars(AVWriter wr, char c, size_t n);
> +
> +/**
> + * Flush data that may be buffered in the writer.
> + */
> +void av_writer_flush(AVWriter wr);
> +
> +/**
> + * Return the error status of the writer, an AVERROR code.
> + *
> + * If self_only is non-zero, return errors only on this writer and not on
> + * possible other writers that it got from the caller. If in doubt, use 0.
> + */
> +int av_writer_get_error(AVWriter wr, int self_only);
> +
> +/**
> + * Print a sane value for invalid input.
> + *
> + * This is a flag for all av_something_write() functions: when presented
> + * with an invalid "something", if the flag is not present they should leave
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...
> + * the writer unchanged; if the flag is present they should print a sane
> + * fallback string.
> + */
> +#define AV_WRITER_FALLBACK 0x1
> +
> +/***/
> +
> +/**
> + * @defgroup av_dynbuf_writer AVDynbufWriter
> + *
>
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Andreas Rheinhardt (12022-08-31): > He is not only stack-allocating AVWriter, he also intends to > stack-allocate the actual writers like AVDynbufWriter, AVBufWriter > (AVWriter is just a wrapper around the underlying writers). This means > that no allocations need to be performed for AVBufWriter at all (and due > to the inherent small-string optimization of an AVBPrint, it can also be > avoided for AVDynbufWriter in lots of cases). > If you return pointers to an AVWriter struct, you need to allocate this > struct somewhere, which means that your init/av_dynbuf_writer_wrap has > an allocation that can fail and therefore needs to be checked; and the > struct needs to be freed lateron. Thanks. I could have been more precise. AVWriter itself uses a new kind of mini decentralized object system, where all objects are a pair of pointers, first a const pointer to the structure of primitive methods and second the structure itself, and are passed by value. It does not require any future extension, it just works that way. This scheme allows to add methods on an existing structure without altering it. So, any kind of side data, for example, can become an object with serialize methods, or an object with ref/unref methods, etc. The various implementations of AVWriter are also designed to be allocated on the stack by the caller. This time, to avoid being encumbered by sizeof(struct) for compatibility, I use a different trick: one of the first field of the structure is its own size, set by whoever allocated it. Currently, the code only checks that the size is at least the original size. If we later extend the structure, the code will have to check if the fields it tries to access are below the reported size, and if they are not it will have to manage without them, use a fallback value. For writers, need for extending the structure is very unlikely. But for the methods structures, it can happen. Methods structures defined by earlier applications will just behave as if these methods are unimplemented. The ability to allocate on the stack is essential to AVWriter, since it needs to be lightweight enough that we never have to hesitate to use it. Of course, it can always be allocated dynamically. I wonder if I should include functions to allocate dynamically and init av_dynbuf_writer() at least. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Leo Izen: > > On 8/30/22 15:37, Nicolas George wrote: >> Leo Izen (12022-08-30): >>> Is there a reason this is AVWriter wr = foo() and not AVWriter *wr = >>> foo()? >>> Most other APIs return pointers to structs, rather than structs >>> themselves >>> (see: av_packet_alloc). Using a pointer would prevent us from having >>> sizeof(AVWriter) as part of the ABI, as was done with AVPacket. >> >> Yes: to return a pointer, you need somewhere to store the structure. One >> of the point of AVWriter is that you can store it on the stack to avoid >> dynamic allocations when the string is short enough. >> >> Note that AVWriter is exactly two pointers. It will always be two >> pointers, and all the objects that I intend to introduce later will >> always be two pointers: one const pointer for the methods, one pointer >> for the object itself. >> >> This design is essential to the features I promised for AVWriter and for >> later. >> > > I don't see how you are planning on avoiding dynamic allocations by > stack-allocating AVWriter when AVWriter itself only contains two pointers. > > I also don't see how this design is essential to the features you > promised in a way that can't be done by just not making sizeof(AVWriter) > part of the ABI and returning pointers to a struct. > He is not only stack-allocating AVWriter, he also intends to stack-allocate the actual writers like AVDynbufWriter, AVBufWriter (AVWriter is just a wrapper around the underlying writers). This means that no allocations need to be performed for AVBufWriter at all (and due to the inherent small-string optimization of an AVBPrint, it can also be avoided for AVDynbufWriter in lots of cases). If you return pointers to an AVWriter struct, you need to allocate this struct somewhere, which means that your init/av_dynbuf_writer_wrap has an allocation that can fail and therefore needs to be checked; and the struct needs to be freed lateron. - 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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On 8/30/22 15:37, Nicolas George wrote: Leo Izen (12022-08-30): Is there a reason this is AVWriter wr = foo() and not AVWriter *wr = foo()? Most other APIs return pointers to structs, rather than structs themselves (see: av_packet_alloc). Using a pointer would prevent us from having sizeof(AVWriter) as part of the ABI, as was done with AVPacket. Yes: to return a pointer, you need somewhere to store the structure. One of the point of AVWriter is that you can store it on the stack to avoid dynamic allocations when the string is short enough. Note that AVWriter is exactly two pointers. It will always be two pointers, and all the objects that I intend to introduce later will always be two pointers: one const pointer for the methods, one pointer for the object itself. This design is essential to the features I promised for AVWriter and for later. I don't see how you are planning on avoiding dynamic allocations by stack-allocating AVWriter when AVWriter itself only contains two pointers. I also don't see how this design is essential to the features you promised in a way that can't be done by just not making sizeof(AVWriter) part of the ABI and returning pointers to a struct. - Leo Izen (thebombzen) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Leo Izen (12022-08-30): > Is there a reason this is AVWriter wr = foo() and not AVWriter *wr = foo()? > Most other APIs return pointers to structs, rather than structs themselves > (see: av_packet_alloc). Using a pointer would prevent us from having > sizeof(AVWriter) as part of the ABI, as was done with AVPacket. Yes: to return a pointer, you need somewhere to store the structure. One of the point of AVWriter is that you can store it on the stack to avoid dynamic allocations when the string is short enough. Note that AVWriter is exactly two pointers. It will always be two pointers, and all the objects that I intend to introduce later will always be two pointers: one const pointer for the methods, one pointer for the object itself. This design is essential to the features I promised for AVWriter and for later. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
On 8/24/22 11:18, Nicolas George wrote:
+```
+AVWriter wr = av_dynbuf_writer();
+av_something_write(wr, something, 0);
+if (av_writer_get_error(wr, 0) < 0)
+die("Failed");
+use_string(av_dynbuf_writer_get_data(wr, NULL));
+av_dynbuf_writer_finalize(wr, NULL, NULL);
+```
Is there a reason this is AVWriter wr = foo() and not AVWriter *wr =
foo()? Most other APIs return pointers to structs, rather than structs
themselves (see: av_packet_alloc). Using a pointer would prevent us from
having sizeof(AVWriter) as part of the ABI, as was done with AVPacket.
- Leo Izen (thebombzen)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Nicolas George (12022-08-24): > The actual implementation, tests and uses in the rest of > FFmpeg code will be committed separately once the API is > settled. > > Signed-off-by: Nicolas George > --- > doc/avwriter_intro.md | 109 ++ > libavutil/writer.h| 484 ++ > 2 files changed, 593 insertions(+) > create mode 100644 doc/avwriter_intro.md > create mode 100644 libavutil/writer.h > > > As suggested by JB, here is the header and documentation for AVWriter, > to discuss the principle before investing time in polishing the > implementation. I expect to discuss the API and if no blockign > objections are raised push it then spend time on the implementation. > > This API is a nicer and much more powerful version of BPrint. It can be > used to simplify existing code where BPrint could help, plus places > where BPrint could not help. > > It also is the prerequisite for more ambitious projects, expecially > universal serialization of FFmpeg objects (side data and such) into > standardized formats. > > The implementation is in most part done, and I am sure I can deliver. > What remains is the boring part of integrating the tests in FATE, > polishing various parts, updating the parts where I changed my mind > midway, etc. > > Note: FF_NEW_SZ is a macro defined elsewhere; it is really part of the > implementation more than the API. > > Ping. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Andreas Rheinhardt (12022-08-24): > Where is this header? # Note: FF_NEW_SZ is a macro defined elsewhere; it is really part of the # implementation more than the API. This is all there is to it for this case. I can move it here for now. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
Nicolas George:
> The actual implementation, tests and uses in the rest of
> FFmpeg code will be committed separately once the API is
> settled.
>
I think you misunderstood JB: He did not say that the headers are pushed
without the implementation, he just said that the headers should be
discussed and approved before you code the actual implementation:
>
> You provide a full header and documentation, and then get it discussed.
> When there is a consensus for approval of the headers, then, you can code the
> core of it that matches the headers.
> That avoids the "I spent time for nothing" issue.
> diff --git a/libavutil/writer.h b/libavutil/writer.h
> new file mode 100644
> index 00..55e2cf3ea6
> --- /dev/null
> +++ b/libavutil/writer.h
> @@ -0,0 +1,484 @@
> +/*
> + * Copyright (c) 2022 The FFmpeg project
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> + */
> +
> +#ifndef AVUTIL_WRITER_H
> +#define AVUTIL_WRITER_H
> +
> +#include
> +#include
> +#include
> +
> +#include "extendable.h"
Where is this header?
> +#include "bprint.h"
> +
> +/**
> + * @defgroup av_writer AVWriter
> + *
> + * Object-oriented API to write strings and binary data.
> + *
> + * @{
> + */
> +
> +typedef struct AVWriterMethods AVWriterMethods;
> +
> +/**
> + * Opaque object to write strings and binary data.
> + *
> + * AVWriter is meant to allow to build and return strings (and blocks of
> + * binary data) efficiently between functions.
> + * For example, a function that serialize something into a string will
> + * actually write into an AVWriter, and the caller will choose the way the
> + * data will be stored or processed on the fly.
> + *
> + * For a quick introduction on how to use AVWriter in simple cases, see
> + * doc/avwriter_intro.md.
> + *
> + * There are various pre-defined types of AVWriter, see below:
> + *
> + * - av_dynbuf_writer() writes the data into a buffer that is grown with
> + * av_realloc() if it does not fit in the initial buffer; it is the
> + * recommended choice.
> + *
> + * - av_buf_writer() writes the data into a pre-existing finite buffer.
> + *
> + * - av_stdio_writer() writes the data into a FILE*.
> + *
> + * - av_log_writer() writes the data to av_log().
> + *
> + * AVWriter objects are passed by value. It allows creating a writer on the
> + * fly, without extra variable or error checking. The structure can never
> + * change.
> + */
> +typedef struct AVWriter {
> +const AVWriterMethods *methods;
> +void *obj;
> +} AVWriter;
> +
> +/**
> + * Write a data buffer to an AVWriter.
> + */
> +void av_writer_write(AVWriter wr, const char *buf, size_t size);
> +
> +/**
> + * Write a 0-terminated string to an AVWriter.
> + */
> +void av_writer_print(AVWriter wr, const char *str);
> +
> +/**
> + * Write a formatted string to an AVWriter.
> + * Note: do not use libc-specific extensions to the format string.
> + */
> +void av_writer_printf(AVWriter wr, const char *fmt, ...);
> +
> +/**
> + * Write a formatted to string an AVWriter using a va_list.
s/to string/string to/
> + */
> +void av_writer_vprintf(AVWriter wr, const char *fmt, va_list va);
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavu: header and documentation for AVWriter
> -Original Message-
> From: ffmpeg-devel On Behalf Of
> Nicolas George
> Sent: Wednesday, August 24, 2022 5:18 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH] lavu: header and documentation for
> AVWriter
>
> The actual implementation, tests and uses in the rest of
> FFmpeg code will be committed separately once the API is
> settled.
>
> Signed-off-by: Nicolas George
> ---
> doc/avwriter_intro.md | 109 ++
> libavutil/writer.h| 484
> ++
> 2 files changed, 593 insertions(+)
> create mode 100644 doc/avwriter_intro.md
> create mode 100644 libavutil/writer.h
>
>
> As suggested by JB, here is the header and documentation for
> AVWriter,
> to discuss the principle before investing time in polishing the
> implementation. I expect to discuss the API and if no blockign
> objections are raised push it then spend time on the implementation.
>
> This API is a nicer and much more powerful version of BPrint. It can
> be
> used to simplify existing code where BPrint could help, plus places
> where BPrint could not help.
>
> It also is the prerequisite for more ambitious projects, expecially
> universal serialization of FFmpeg objects (side data and such) into
> standardized formats.
>
> The implementation is in most part done, and I am sure I can deliver.
> What remains is the boring part of integrating the tests in FATE,
> polishing various parts, updating the parts where I changed my mind
> midway, etc.
>
> Note: FF_NEW_SZ is a macro defined elsewhere; it is really part of
> the
> implementation more than the API.
>
>
> diff --git a/doc/avwriter_intro.md b/doc/avwriter_intro.md
> new file mode 100644
> index 00..4fd8a5a4ad
> --- /dev/null
> +++ b/doc/avwriter_intro.md
> @@ -0,0 +1,109 @@
> +# Quick start guide for AVWriter
> +
> +AVWriter is an API to unify functions returning strings (or any
> arbitrary
> +binary buffer) and to make building strings from parts easier. Here
> is a
> +quick introduction with pairs of “what I would do without AVWriter”
> /
> +“how to do it with AVWriter” of example code.
> +
> +## I want a `char*` buffer, the function wants an AVWriter
> +
> +Old-style code:
> +
> +```
> +char *buf;
> +ret = av_something_to_string(, something);
> +if (ret < 0)
> +die("Failed");
> +use_string(buf);
> +av_freep();
> +```
> +
> +Equivalent code with AVWriter:
> +
> +```
> +AVWriter wr = av_dynbuf_writer();
> +av_something_write(wr, something, 0);
> +if (av_writer_get_error(wr, 0) < 0)
> +die("Failed");
Will it be possible to do:
av_something_write(wr, something1, 0);
av_something_write(wr, something2, 0);
av_something_write(wr, something3, 0);
if (av_writer_get_error(wr, 0) < 0)
die("Failed");
or would av_writer_get_error() need to be called after each
av_something_write()?
Thanks,
softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
