[FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

Hi Stefano, Andreas, Nicolas
and of course, anybody who's interested in the AVTextFormat APIs,


let me start by saying that I have no intention to rush the 
publicization of those APIs. I think there's still a way to go.
But it's also true that when you don't have a clear understanding
of where you actually want to go, you'll hardly arrive there.

At the implementation level, I sensed that "you" ("FFmpeg")
are following some principles which are somewhat contradictive to 
those that I'm usually adhering to (e.g. "parameter validation
being a responsibility of the call site, crashing otherwise 
being acceptable"). Nonetheless, I'm the one who has to adapt,
and I'm not going to question that.

I'm addressing this to the three of you, due to your review 
comments and while I wouldn't call them contradictive, these
are heading into slightly different directions (removing 
checks altogether, replacing with assertions or printing a log
message in cases).
The one personal thought that I'd like to add, is that I 
think that parameter treatment (which checks to apply
or not) should depend on the access level of a specific API,
whether it's public, internal or private. That's something
where we don't have a plan for yet.

The AVTextFormatContext structure is all open at the moment,
means it gives everybody access to everything:

- You can call the text format api functions
- You can access the section definitions, the BPrint buffers
  and all other state and config values
- You can access and invoke the AVTextFormatter directly via
  its registered functions
- You can access AVTextWriterContext and the writers
  directly, again allowing direct invocation of their 
  functions

I did that intentionally to avoid any blockage during 
migration and also for being in a position later (=now)
where everything remains doable. 
Now that we've approached at this point, it would probably
be good to come to a clearer picture of the intended 
directions for the API.
Based on that, it should be easier to determine consistent
patterns for parameter validation for each category of API
functions - then I can apply that once and for all, 
cleanly and consistently everywhere.


This is how I'd categorize the current functions:


AVTextFormatter Implementations

- print_section_header
- print_section_footer
- print_integer
- print_string

(default, compact, csv, json, xml, ini, etc.)


AVTextWriter Implementations

- writer_w8
- writer_put_str
- writer_printf

(avio, stdout, buffer)
        

TextFormat API

- avtext_context_open
- avtext_context_close
- avtext_print_section_header
- avtext_print_section_footer
- avtext_print_integer
- avtext_print_integer_flags
- avtext_print_string
- avtext_print_unit_int
- avtext_print_rational
- avtext_print_time
- avtext_print_ts
- avtext_print_data
- avtext_print_data_hash
- avtext_print_integers
- avtext_get_formatter_by_name


TextWriter API

- avtextwriter_context_open
- avtextwriter_context_close
- avtextwriter_create_stdout
- avtextwriter_create_avio
- avtextwriter_create_file
- avtextwriter_create_buffer


Please let me know what you think,
thanks,
sw


_______________________________________________
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] On errors, asserts and crashing (was: Shaping the AVTextFormat API Surface)

[Nicolas George]

softworkz . (HE12025-04-22):
> At the implementation level, I sensed that "you" ("FFmpeg")
> are following some principles which are somewhat contradictive to 
> those that I'm usually adhering to (e.g. "parameter validation
> being a responsibility of the call site, crashing otherwise 
> being acceptable"). Nonetheless, I'm the one who has to adapt,
> and I'm not going to question that.

I am feeling didactic, so I will write what I have to say on the topic
in full details, so that it can also serve as future reference.

Crashing immediately when some unexpected condition happens is not a
quirk of a few FFmpeg developers, it is very often the only way to do
things and also often better than any available alternative. Knowing
what to do when it is only an option is not always obvious, but this is
FFmpeg, not some management software project developed by engineers
fresh from an average school.

Consider the simplest mathematical operation that can fail, a division:

	a = b / c;

What happens if c is 0?

It depends: on the language, the CPU, the build options, possibly the
phases of the Moon. But mostly, what happens lives between two extremes:

- it crashes immediately, sometimes even with a hardware exception
  generated by the CPU;

- it returns some kind of cheesy not-a-number value that will
  contaminate all computations done with a, leading to silently
  corrupted output.

Intermediate cases can be: return a not-a-number but something later
tests it and crashes; or: return not a number and be later conditionally
discarded to produce corrupt output only once in a while. For the sake
of simplicity, let us focus on the extreme cases.

There are two things to say here:

- both are very bad;

- silently corrupting the output is the worst.

(There are languages where division by zero has a precise and
mathematically consistent semantic that a very careful programmer could
make use of. That does not invalidate my point.)

Both are bad: that means that whenever we write a division, we must be
sure that the divisor is not zero. Sometimes, it is very easy:

	unsigned n = 0;
	do { n++; } while (*(p++));
	a = b / n;

Loops of the do…while kind are always executed at least once, therefore
we know that n ≠ at the end. (Did you notice the flaw in that reasoning?
I will come back to it later.)

Sometimes, it is just a matter of testing:

	if (nb_notes == 0)
	    die("Why are you asking me to compute an average when you
	         have not yet entered any note, you idiot?");

And sometimes it relies on a 10-pages mathematical proof that the
algorithm we are implementing is correct.

But hard or easy, it is always the responsibility of the programmer to
know that the divisor is not zero before coding a division.

This is not unique to divisions:

We must know a pointer points to a valid object before dereferencing it.

We must know a pointer points to writable memory before writing to it.

We must know an index is within the boundaries of an array before
accessing it.

And this is not limited to C and other programs FFmpeg is fond of: write
into a cell beyond the end of the array in C, it will let you modify
something else entirely and that might let the spooks or cryptlockers
in; but do it in Java or OCaml, it will trigger an exception that we
probably neglected to catch, because if we thought of catching it we
would probably have written it more elegantly.

The conclusion of this section is this:

Any programmer worth their salt must know a lot of invariants about each
line of their code, invariants that are not written into the code but
that they would be able to express and prove if asked to. “At this
point, this number is not zero”; “at this point, this pointer comes from
malloc() or realloc()”, “at this point there are no other references to
this object”, “at this point this block of memory has a 0 before the
end” and so on.

Failure to account for the invariants required by the constructs used in
the program will result in the program misbehaving in more or less
harmful ways.


Now about function calls and API.

As I said, constructs in the program have invariant requirements: if we
divide, it must not be by zero. These requirements propagate backwards
along the flow of the program: if l must not be zero, then the string
it is the length of must not be empty, therefore the list from which it
is built must not itself be empty, and so on.

A program behaves correctly if at all places in the code flow, the
invariants guaranteed by the earlier parts are at least as strict as the
invariant requirements impose by the later parts.

In a complete program, functions are just roudabouts in the program
flow. As such, like any place in the program there are invariant
requirements. But if we think of functions as separating different parts
of a program that are under the responsibility of different person
(or the same person at different times), theses invariants requirements
must be documented.

Sometimes, it is very subtle: “The strlen function computes the length
of the string pointed to by s”: where does it say s must not be a NULL
pointer? Not explicitly, but it is implied by “string pointed to”,
because NULL does not point to anything, let alone a string.

What happens if we call strlen() on a NULL pointer? That depends. On
Unix systems, it usually results in a SIGSEGV; something similar will
happen with other memory-protected OSes. But on a microcontroller, it
will just compute how many non-0 octets there are at this place in the
memory. Whatever the effect, it misbehaves.


What about mistakes?

Even excellent programmers, programmers worth rose Himalayan salt, can
make mistakes. For example, in the code I gave above as an example where
it is easy to be sure that n is not zero, what happens on x86_64 if p
points to four giga-octets of non-zero values and then a 0? Oops.

In the ideal world, that would be impossible. In the ideal world, the
compiler would refuse to compile a program unless it can prove the
required invariants are met, with the help of annotations helpfully
provided by the programmer.

This is what Rust tried to do and lamentably failed at.

(That would leave out programs used to explore unprovable mathematical
properties. I am perfectly fine letting them use some kind of
“ignore-invariant” pragma, as long as it is not used in a program that
might crash a space probe, explode a nuclear plant or, worse, make a
game unwinnable.)

In the real world, we are not there yet, by far. Yet we must decide what
to do about all these invariants. In most of the places, we do nothing,
because the road is straight and if we are speeding, the speed bump a
hundred meters from now will trash us. But it is still useful to add our
own speed bumps and traffic lights at strategic places.

The worst thing we could do is pump the brakes before the speed bump. It
is called “defensive programming”: “at this point, this value should not
be negative, let us check if it is and make it 0 if necessary, the show
must go on!” Well, it is not a show, it is a program producing a result,
and if it goes on after breaking one of its invariant requirements it
will almost certainly produce invalid result. That is evil.


But do we add traffic lights or a speed bump? My traffic analogy breaks
down, I will stop using it. Do we add do nothing, do we an assert or do
we return an error?

But first, a short point about asserts. Logically, an assert and doing
nothing are the same, because asserts are often removed at build time
(by the way, I hope all FFmpeg developers remember to always work with
the maximum assert level). And because “crashing right now” is one case
of “anything can happen”. But asserts, when they are enabled, make
mistakes easier to debug, because they make their consequences more
visible earlier.

So: do nothing / assert, or return an error? This is the point where
there is no longer an easy answer. It requires weighting the pros and
the cons. The only thing I can do is make a list of a few considerations
that affects the weighting of the pros and cons.

Mostly, I will talk about code wrapped inside a function, with the
caller possibly under the responsibility of a different programmer.

One of the most important ones is: Is there already a channel to return
an error?

If the caller is already expected to check for an error return due to
conditions outside of the control of any programmer (the hard drive
died!), then using it to signal a programming error is an easy option.
On the other hand, if the function has been of type void for ten years,
changing it to return an error now is almost certainly not a good idea,
because a lot of code already calls it without checking for an error.

Even worse if the function used to return a positive value that was not
supposed to be an error, and now we want to return an error with a
distinct set of value (negative AVERROR code): we are in the “cheesy
not-a-number value” case and risk contaminating the output.

Another important one is: How easy is it to ensure the requirement is
met?

“Pointer must not be null” is one of the easiest to ensure, and as a
consequence it is one that is very often completely neglected, relying
on the OS sending a SIGSEGV.

“Flags must be a binary OR of the following constants” is another very
easy one: just do not do any fancy arithmetic with the value that will
end up in the flags. If we only use the designated constants and binary
arithmetic, we are guaranteed to be good. If not, what are we doing
exactly?

On the other hand, “expr must point to a string with a syntactically
valid expression”: I cannot check the syntax of expr without duplicating
a large part of the code I am about to call, and sometimes I might want
to use an expression that comes from the outside.

Useful rule of thumb: What would the C language do?

When writing a new function, we can try to imagine how it would work if
was a standard C function, and for that look at similar cases in the
libc. For example, many many C functions say “undefined behavior” if
NULL is passed where NULL is not expected. Whenever the answer to “what
would the C language do” contains the words “undefined behavior”, then
the best choice is to do nothing, possibly with an assert.

System calls, on the other hand, are not a very good rule of thumb: they
are heavily skewed towards “everything returns an error code, it is a
grave mistake to ignore it”; they are heavily burdened by the need for
compatibility with existing code and other systems; and the consequences
of doing nothing and just let the dice fall would be catastrophic.


To summarize: Deciding what to do when invalid values are met requires
properly understanding the invariant requirements of the program and
doing a cost-analysis of the options, including whether the rest of the
code can easily enforce those invariants and whether it can do something
graceful with an error.

Advice addressed are more beginner programmers tend to be more
one-size-fit-all, and as such inefficient. And often more dogmatic. 

Regards,

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing (was: Shaping the AVTextFormat API Surface)

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Donnerstag, 24. April 2025 16:47
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: [FFmpeg-devel] On errors, asserts and crashing (was: Shaping
> the AVTextFormat API Surface)
> 
> softworkz . (HE12025-04-22):
> > At the implementation level, I sensed that "you" ("FFmpeg")
> > are following some principles which are somewhat contradictive to
> > those that I'm usually adhering to (e.g. "parameter validation
> > being a responsibility of the call site, crashing otherwise
> > being acceptable"). Nonetheless, I'm the one who has to adapt,
> > and I'm not going to question that.
> 
> I am feeling didactic, so I will write what I have to say on the topic
> in full details, so that it can also serve as future reference.
> 
> Crashing immediately when some unexpected condition happens is not a
> quirk of a few FFmpeg developers, it is very often the only way to do
> things and also often better than any available alternative. Knowing
> what to do when it is only an option is not always obvious, but this
> is
> FFmpeg, not some management software project developed by engineers
> fresh from an average school.

Once and for all, just GTFO from those kinds of comments. I do have 
an academic background and the university were considered to
be among the top 10 in the world for Computer Science at that time.
The only reason why I haven't become "Dr. Softworkz" is because I 
had no interest in teaching other students and I already had a running
business with earnings way above, hence I had rejected the offer.

I'm showing interest in your views of parameter validation in the 
context of Ffmpeg - nothing less, but also nothing more, so we 
can skip fundamentals.

[…]


> So: do nothing / assert, or return an error? This is the point where
> there is no longer an easy answer. 

Which is why I'm asking about it.


> It requires weighting the pros and
> the cons. The only thing I can do is make a list of a few
> considerations
> that affects the weighting of the pros and cons.

Sure. But what I've been asking about are some very specific 
cases only.

> To summarize: Deciding what to do when invalid values are met requires
> properly understanding the invariant requirements of the program and
> doing a cost-analysis of the options, including whether the rest of
> the
> code can easily enforce those invariants and whether it can do
> something
> graceful with an error.

I have nothing to object about your elaboration, besides that it's 
missing out on the concept of structured exception handling, which is
an intrinsic feature of many programming languages, offering the 
undeniable advantage of being able to recover from "exceptional"
situations instead of crashing.

Thanks
sw




_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing (was: Shaping the AVTextFormat API Surface)

[Nicolas George]

softworkz . (HE12025-04-25):
> Once and for all, just GTFO from those kinds of comments.

That was not directed at you. But you have now lost all rights to a
cordial reply from me with this rudeness.

>							    I do have 
> an academic background and the university were considered to
> be among the top 10 in the world for Computer Science at that time.
> The only reason why I haven't become "Dr. Softworkz" is because I 
> had no interest in teaching other students and I already had a running
> business with earnings way above, hence I had rejected the offer.

Just so you know, PhD programs never taught anybody to do a good
developer. They are about research, which is an entirely different
thing. If some doctors are excellent developers — I know a few examples
— it is because they learned by themselves before or in parallel.

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing (was: Shaping the AVTextFormat API Surface)

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Freitag, 25. April 2025 16:05
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] On errors, asserts and crashing (was:
> Shaping the AVTextFormat API Surface)
> 
> softworkz . (HE12025-04-25):
> > Once and for all, just GTFO from those kinds of comments.
> 
> That was not directed at you. But you have now lost all rights to a
> cordial reply from me with this rudeness.

You are including those kinds of comments almost every single time when
you are responding to me and I'm just sick of it:

> this is
> FFmpeg, not some management software project developed by engineers
> fresh from an average school.


It doesn't matter how it could be nitpickingly interpreted otherwise.

Just stop this and we can communicate friendly and respectfully.


sw

_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing (was: Shaping the AVTextFormat API Surface)

[Nicolas George]

softworkz . (HE12025-04-25):
> You are including those kinds of comments almost every single time when
> you are responding to me and I'm just sick of it:

I include this kind of comments whenever the occasion presents itself,
whoever I am replying to. Check your narcissism.

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing (was: Shaping the AVTextFormat API Surface)

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Freitag, 25. April 2025 16:42
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] On errors, asserts and crashing (was:
> Shaping the AVTextFormat API Surface)
> 
> softworkz . (HE12025-04-25):
> > You are including those kinds of comments almost every single time
> when
> > you are responding to me and I'm just sick of it:
> 
> I include this kind of comments whenever the occasion presents itself,
> whoever I am replying to. Check your narcissism.
> 
> --

Says the king or narcissism. 😃 
_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 143 bytes --]

On 4/25/2025 10:05 AM, softworkz . wrote:
> Once and for all, just GTFO from those kinds of comments

This kind of aggression is not ok.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> James Almer
> Sent: Freitag, 25. April 2025 16:43
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] On errors, asserts and crashing
> 
> On 4/25/2025 10:05 AM, softworkz . wrote:
> > Once and for all, just GTFO from those kinds of comments
> 
> This kind of aggression is not ok.

I agree that it was a bit over the top, but still well-deserved
for the repetitive disrespectful responses.

sw

_______________________________________________
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".

Re: [FFmpeg-devel] On errors, asserts and crashing

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 990 bytes --]

On Fri, Apr 25, 2025 at 02:49:33PM +0000, softworkz . wrote:
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > James Almer
> > Sent: Freitag, 25. April 2025 16:43
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] On errors, asserts and crashing
> > 
> > On 4/25/2025 10:05 AM, softworkz . wrote:
> > > Once and for all, just GTFO from those kinds of comments
> > 
> > This kind of aggression is not ok.
> 
> I agree that it was a bit over the top, but still well-deserved
> for the repetitive disrespectful responses.

I think, the "On errors, asserts and crashing"
should have been a seperate and new thread. Its a good text
its not a good reply IMHO.

thx


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Nicolas George]

softworkz . (HE12025-04-22):
> AVTextFormatter Implementations
> 
> - print_section_header
> - print_section_footer
> - print_integer
> - print_string

> TextFormat API
> 
> - avtext_context_open
> - avtext_context_close
> - avtext_print_section_header
> - avtext_print_section_footer

You are ignoring one of the main reasons I gave: this API is far from
acceptable as is in libavutil because it is way too specific to ffprobe.

ffprobe has a concept of sections, and no more. XML does not have a
concept of sections. JSON does not have a concept of sections. CSV does
not have a concept of sections. Other parts of FFmpeg
that could benefit from it do not, or they may have subsection,
subsubsections, etc. Applications that may use this API even more so.

The proper way to go at it involves two steps. These steps might
overlap, but not by much. The first one is rather easy but long. The
second one can be quick but it is much harder.


The first step is adding each format into libavutil separately, with
distinct APIs tailored for the specificities of each format. The APIs
should run parallel whenever possible, i.e. use similar names and
prototypes for things that make sense in multiple contexts. But other
parts will be completely unique to certain formats.

So:

av_json_enc_…(): adding objects (dictionaries), arrays, strings, numbers,
booleans, null values; controlling the indentation, the kind of quotes,
the encoding.

av_xml_enc_…(): similar, but: no concept of numbers, booleans, null;
and: control over attributes / nested elements, CDATA sections,
comments.

av_csv_enc_…()…

For each API, the parts of ffmpeg already do the same should be
converted to use it. That means the ffprobe writers of course, but not
only. If the XML writing code is not usable by dashenc, movenc,
smoothstreamingenc, vf_signature, ttmlenc, etc., back to the design
step.

We might skip that for the formats that are entirely invented here. Or
some of them.


The second step is designing a common interface for all the formats.
That means finding an almost-common denominator to all the formats,
finding a way to express it even in formats that are not powerful
enough, but also finding ways to tweak the output when the format is
more powerful.

This is hard. I have some ideas on what the common API needs to look
like to encompass XML and JSON, and I think that with those two done the
others could be made to work, bit it is not as clear in my mind as other
plans I have.

Anyway, the first step is already enough work to occupy somebody for
some time.


Also: these APIs can end up being used by things like the showinfo
filters, and called once per frame. That means they must be fast, and in
particular they should not need dynamic allocations as long as the
objects are small.

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Donnerstag, 24. April 2025 19:12
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> softworkz . (HE12025-04-22):
> > AVTextFormatter Implementations
> >
> > - print_section_header
> > - print_section_footer
> > - print_integer
> > - print_string
> 
> > TextFormat API
> >
> > - avtext_context_open
> > - avtext_context_close
> > - avtext_print_section_header
> > - avtext_print_section_footer
> 
> You are ignoring one of the main reasons I gave: this API is far from
> acceptable as is in libavutil because it is way too specific to
> ffprobe.

Now it rather seems that you are ignoring that I have explicitly 
acknowledged that, by saying that in this form, it is not yet ready 
for becoming a public API in avutil, so we are kind of on the same
page in this regard, albeit you seem to have some more radical changes
in mind than I do, but that's fine - let's talk about it!


> ffprobe has a concept of sections, and no more. XML does not have a
> concept of sections. JSON does not have a concept of sections. CSV
> does
> not have a concept of sections. Other parts of FFmpeg
> that could benefit from it do not, or they may have subsection,
> subsubsections, etc. Applications that may use this API even more so.

Sure, neither XML, nor JSON nor YML formats have concepts which are
the same like the sections of the text formatting APIs.

But right now, the sections-concept already maps "more-or-less" 


> 
> The proper way to go at it involves two steps. These steps might
> overlap, but not by much. The first one is rather easy but long. The
> second one can be quick but it is much harder.
> 
> 
> The first step is adding each format into libavutil separately, with
> distinct APIs tailored for the specificities of each format. The APIs
> should run parallel whenever possible, i.e. use similar names and
> prototypes for things that make sense in multiple contexts. But other
> parts will be completely unique to certain formats.
> 
> So:
> 
> av_json_enc_…(): adding objects (dictionaries), arrays, strings,
> numbers,
> booleans, null values; controlling the indentation, the kind of
> quotes,
> the encoding.
> 
> av_xml_enc_…(): similar, but: no concept of numbers, booleans, null;
> and: control over attributes / nested elements, CDATA sections,
> comments.
> 
> av_csv_enc_…()…
> 
> For each API, the parts of ffmpeg already do the same should be
> converted to use it. That means the ffprobe writers of course, but not
> only. If the XML writing code is not usable by dashenc, movenc,
> smoothstreamingenc, vf_signature, ttmlenc, etc., back to the design
> step.
> 
> We might skip that for the formats that are entirely invented here. Or
> some of them.
> 
> 
> The second step is designing a common interface for all the formats.
> That means finding an almost-common denominator to all the formats,
> finding a way to express it even in formats that are not powerful
> enough, but also finding ways to tweak the output when the format is
> more powerful.
> 
> This is hard. I have some ideas on what the common API needs to look
> like to encompass XML and JSON, and I think that with those two done
> the
> others could be made to work, bit it is not as clear in my mind as
> other
> plans I have.
> 
> Anyway, the first step is already enough work to occupy somebody for
> some time.
> 
> 
> Also: these APIs can end up being used by things like the showinfo
> filters, and called once per frame. That means they must be fast, and
> in
> particular they should not need dynamic allocations as long as the
> objects are small.
> 
> --
>   Nicolas George
> _______________________________________________
> 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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

My previous response got sent out too early by accidence.
This is the complete one.

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Donnerstag, 24. April 2025 19:12
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> softworkz . (HE12025-04-22):
> > AVTextFormatter Implementations
> >
> > - print_section_header
> > - print_section_footer
> > - print_integer
> > - print_string
> 
> > TextFormat API
> >
> > - avtext_context_open
> > - avtext_context_close
> > - avtext_print_section_header
> > - avtext_print_section_footer
> 
> You are ignoring one of the main reasons I gave: this API is far from
> acceptable as is in libavutil because it is way too specific to
> ffprobe.

Now it rather seems that you are ignoring that I have explicitly 
acknowledged that, by saying that in this form, it is not yet ready 
for becoming a public API in avutil, so we are kind of on the same
page in this regard, albeit you seem to have some more radical changes
in mind than I do, but that's fine - let's talk about it!


> ffprobe has a concept of sections, and no more. XML does not have a
> concept of sections. JSON does not have a concept of sections. CSV
> does
> not have a concept of sections. Other parts of FFmpeg
> that could benefit from it do not, or they may have subsection,
> subsubsections, etc. Applications that may use this API even more so.

Sure, neither XML, nor JSON nor YML formats have concepts which are
the same like the sections of the text formatting APIs.

But right now, the sections-concept already maps "more-or-less" 
well to XML and JSON constructs.

It also mapped surprisingly well to the Mermaid diagram format,
which is why I wouldn't consider it that bad.


> The second step is designing a common interface for all the formats.
> That means finding an almost-common denominator to all the formats,
> finding a way to express it even in formats that are not powerful
> enough, but also finding ways to tweak the output when the format is
> more powerful.

Does it really need to be something entirely different from the 
current sections concept?

I'm not sure about that part - I was thinking that it could be 
refined to better map to XML and JSON formats..


Thanks for your thoughts,
sw

_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Nicolas George]

softworkz . (HE12025-04-25):
> Does it really need to be something entirely different from the 
> current sections concept?

Yes.

And it is all the answer you will get from me after your rudeness.

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Freitag, 25. April 2025 16:05
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> softworkz . (HE12025-04-25):
> > Does it really need to be something entirely different from the
> > current sections concept?
> 
> Yes.
> 
> And it is all the answer you will get from me after your rudeness.
> 
> --

I'm always ever just mirroring your own rudeness. The start is 
always on your side and you just get the responses that you 
deserve.

Address me in a friendly and respectful way and I'll always 
respond to you in the same way. 

Pretty simple, isn't it?`

sw
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Stefano Sabatini]

On date Thursday 2025-04-24 19:12:09 +0200, Nicolas George wrote:
> softworkz . (HE12025-04-22):
[...]
> ffprobe has a concept of sections, and no more. XML does not have a
> concept of sections. JSON does not have a concept of sections. CSV does
> not have a concept of sections. Other parts of FFmpeg
> that could benefit from it do not, or they may have subsection,
> subsubsections, etc. Applications that may use this API even more so.

Elaborating on this. ffprobe/textformat is based on a notion of
hierarchical tree-like data, which maps pretty well with most data
formats, at the price that there are some ambiguities which need to be
resolved.

For example if you consider an XML element, it is represented as a
node (aka section): attributes might be represented as key-value
fields, as child nodes (subsections) with the key being the name of
the section and the value its datum, or as a list of key-value
elements.

This data representation choice is embodied in the definition of the
structure to fill, so there is no way to control serialization
"dynamically", but that was not the purpose of that API, since at the
end what we want is to be able to deserialize specific data rather
than being able to serialize any possible data using a specific
container format.

Considering this, there is probably no need to extend the API to cover
each possible format full semantics - this at least it is my view.
 
> The proper way to go at it involves two steps. These steps might
> overlap, but not by much. The first one is rather easy but long. The
> second one can be quick but it is much harder.
> 
> 
> The first step is adding each format into libavutil separately, with
> distinct APIs tailored for the specificities of each format. The APIs
> should run parallel whenever possible, i.e. use similar names and
> prototypes for things that make sense in multiple contexts. But other
> parts will be completely unique to certain formats.
> 
> So:
> 
> av_json_enc_…(): adding objects (dictionaries), arrays, strings, numbers,
> booleans, null values; controlling the indentation, the kind of quotes,
> the encoding.
> 
> av_xml_enc_…(): similar, but: no concept of numbers, booleans, null;
> and: control over attributes / nested elements, CDATA sections,
> comments.
> 
> av_csv_enc_…()…
>
> For each API, the parts of ffmpeg already do the same should be
> converted to use it. That means the ffprobe writers of course, but not
> only. If the XML writing code is not usable by dashenc, movenc,
> smoothstreamingenc, vf_signature, ttmlenc, etc., back to the design
> step.

As I wrote, this was not the purpose of the ffprobe formats in the
first place. MOV/DASH requires a specific use of an XML encoder. In
theory it might be done using the textformat API in the current form,
but it would be probably pretty awkward. We might want to factorize a
few generic utilities (e.g. escaping) to avoid code duplication
though.

For the filters output data we also might need to define the output
structure - currently the output is very simple therefore simple
definitions should be good enough - while serialization performance is
probably not a real concern.

> Also: these APIs can end up being used by things like the showinfo
> filters, and called once per frame. That means they must be fast, and in
> particular they should not need dynamic allocations as long as the
> objects are small.

This is a good point, but again probably the performance at this stage
for the current usage (ffprobe format, filters, etc.) is not a real
concern.

It might be if we want to make this a generic tool for library users,
and for purposes outside of the scope for which it was designed. But I
don't think this should be a real blocker - and we might even keep the
API private to enable libav* cross-librariers but not
external-libraries usage.
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Nicolas George]

Stefano Sabatini (HE12025-04-27):
> Elaborating on this. ffprobe/textformat is based on a notion of
> hierarchical tree-like data

No, this is not true. FFprobe and all its supporting code is based on
one level of hierarchy. Just one level, not a real hierarchical
structure.

> Considering this, there is probably no need to extend the API to cover
> each possible format full semantics - this at least it is my view.

If we add XML-writing code in libavutil, it must be usable for all the
places we are already producing XML. Otherwise it does not make sense.

> As I wrote, this was not the purpose of the ffprobe formats in the
> first place. MOV/DASH requires a specific use of an XML encoder. In
> theory it might be done using the textformat API in the current form,
> but it would be probably pretty awkward. We might want to factorize a
> few generic utilities (e.g. escaping) to avoid code duplication
> though.

You are going at it backwards.

The goal is not to cram this text writers API forcefully into libavutil
and then see if it might serve other purposes, which is what softworkz
is doing.

The API must be able to handle all our use cases from the start. Until
then, it goes back to the design board.

> It might be if we want to make this a generic tool for library users,
> and for purposes outside of the scope for which it was designed. But I
> don't think this should be a real blocker - and we might even keep the
> API private to enable libav* cross-librariers but not
> external-libraries usage.

I can concede making a generic API for external users is not blocking.
Reluctantly.

But I will not budge on internal uses: this API must serve all our
current use cases or it stay outside the libraries.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Dienstag, 29. April 2025 10:31
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> Stefano Sabatini (HE12025-04-27):
> > Elaborating on this. ffprobe/textformat is based on a notion of
> > hierarchical tree-like data
> 
> No, this is not true. FFprobe and all its supporting code is based on
> one level of hierarchy. Just one level, not a real hierarchical
> structure.
> 
> > Considering this, there is probably no need to extend the API to cover
> > each possible format full semantics - this at least it is my view.
> 
> If we add XML-writing code in libavutil, it must be usable for all the
> places we are already producing XML. Otherwise it does not make sense.
> 
> > As I wrote, this was not the purpose of the ffprobe formats in the
> > first place. MOV/DASH requires a specific use of an XML encoder. In
> > theory it might be done using the textformat API in the current form,
> > but it would be probably pretty awkward. We might want to factorize a
> > few generic utilities (e.g. escaping) to avoid code duplication
> > though.
> 
> You are going at it backwards.
> 
> The goal is not to cram this text writers API forcefully into libavutil
> and then see if it might serve other purposes, which is what softworkz
> is doing.

No, this is not my intention. I thought I had laid it out a number of 
times already, but in case it didn't come through clearly, let me
reiterate the agenda that I'm following and proposing:


1. Extract the text formatting from ffprobe.c and transform it into
   a generic API

2. Give that API a temporary home inside fftools as a staging area

3. Refine and polish the API inside fftools while adding additional
   use cases (like graph printing)

4. Brainstorm and discuss which further changes and refinements are 
   needed to serve all the use cases that we are envisioning for the
   future and apply those changes

5. Move it to avutil once we believe that it's ready for this


=> 1 and 2 are done, 3 is in progress

I'm not proposing to do 5 before 4.


> The API must be able to handle all our use cases from the start. Until
> then, it goes back to the design board.

I never had any different intentions, albeit it can happen that there's
some very specific case that it would not be able to serve.

But it should be able to serve the vast majority of cases, that goes
without saying.


A fundamental point from my point of view though, is that the API 
must remain to be built around the sections concept at its core. It's
a strong concept that enables the exchangeability of the text formatters
independent from the individual use case. It has a number of
shortcomings (as has been mentioned already) in terms of how certain
data maps to the schema of individual formats, but there's sufficient
room to extend the sections concept in a way that makes it possible 
to get better control of the way how it is represented in certain 
output formats (like XML).

Being able to add new formats in a way that these become immediately 
available for all use cases is a high value that must not be
sacrificed. Not every format makes sense for all use cases, but
the ability to use all formats without any (or just little) extra
work is (and must remain to be) a key capability of the API.

A YML formatter is already in the pipeline and another very interesting
area is generation of charts in a similar way like the mermaid 
diagram formatter.

In that regard I consider it counter-productive to start any new
work that is focused on specific formats (xml, json) only. Any work
for improvement should rather be done on the current formatters
and the existing API (or at least should be compatible with it).

Best regards
sw


_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
> Sent: Dienstag, 29. April 2025 20:07
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> > George
> > Sent: Dienstag, 29. April 2025 10:31
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> >
> > Stefano Sabatini (HE12025-04-27):
> > > Elaborating on this. ffprobe/textformat is based on a notion of
> > > hierarchical tree-like data
> >
> > No, this is not true. FFprobe and all its supporting code is based on
> > one level of hierarchy. Just one level, not a real hierarchical
> > structure.
> >
> > > Considering this, there is probably no need to extend the API to cover
> > > each possible format full semantics - this at least it is my view.
> >
> > If we add XML-writing code in libavutil, it must be usable for all the
> > places we are already producing XML. Otherwise it does not make sense.
> >
> > > As I wrote, this was not the purpose of the ffprobe formats in the
> > > first place. MOV/DASH requires a specific use of an XML encoder. In
> > > theory it might be done using the textformat API in the current form,
> > > but it would be probably pretty awkward. We might want to factorize a
> > > few generic utilities (e.g. escaping) to avoid code duplication
> > > though.
> >
> > You are going at it backwards.
> >
> > The goal is not to cram this text writers API forcefully into libavutil
> > and then see if it might serve other purposes, which is what softworkz
> > is doing.
> 
> No, this is not my intention. I thought I had laid it out a number of
> times already, but in case it didn't come through clearly, let me
> reiterate the agenda that I'm following and proposing:
> 
> 
> 1. Extract the text formatting from ffprobe.c and transform it into
>    a generic API
> 
> 2. Give that API a temporary home inside fftools as a staging area
> 
> 3. Refine and polish the API inside fftools while adding additional
>    use cases (like graph printing)
> 
> 4. Brainstorm and discuss which further changes and refinements are
>    needed to serve all the use cases that we are envisioning for the
>    future and apply those changes
> 
> 5. Move it to avutil once we believe that it's ready for this
> 
> 
> => 1 and 2 are done, 3 is in progress
> 
> I'm not proposing to do 5 before 4.
> 
> 
> > The API must be able to handle all our use cases from the start. Until
> > then, it goes back to the design board.
> 
> I never had any different intentions, albeit it can happen that there's
> some very specific case that it would not be able to serve.
> 
> But it should be able to serve the vast majority of cases, that goes
> without saying.
> 
> 
> A fundamental point from my point of view though, is that the API
> must remain to be built around the sections concept at its core. It's
> a strong concept that enables the exchangeability of the text formatters
> independent from the individual use case. It has a number of
> shortcomings (as has been mentioned already) in terms of how certain
> data maps to the schema of individual formats, but there's sufficient
> room to extend the sections concept in a way that makes it possible
> to get better control of the way how it is represented in certain
> output formats (like XML).
> 
> Being able to add new formats in a way that these become immediately
> available for all use cases is a high value that must not be
> sacrificed. Not every format makes sense for all use cases, but
> the ability to use all formats without any (or just little) extra
> work is (and must remain to be) a key capability of the API.



Additional thoughts:

[Stefano]
> > Considering this, there is probably no need to extend the API to cover
> > each possible format full semantics - this at least it is my view.
[Nicolas]
> If we add XML-writing code in libavutil, it must be usable for all the
> places we are already producing XML. Otherwise it does not make sense.


This illustrates the kind of misunderstanding that appears to
exist, and in this regard, I think, it's important to clear up about 
which kind of API we are talking about and which purposes it is meant
to serve and which not:


The AVTextFormat feature allows to create output data in a wide range
of formats from a generic API that is agnostic to the format.
It is not a format-specific API providing full-featured formatting for
any format.
That's the nature of this API and it won't change as it cannot change.


> it must be usable for all the
> places we are already producing XML. Otherwise it does not make sense

How do you come to that idea? It's a generic, format-agnostic 
text formatting API. It is not an XML API.

I didn't even get it in the first place that that's what you mean,
it's somewhat far off... Hence I also need to clarify this:
Where I said above that it should be able to cover all use cases, I 
meant "all use cases that need multi-format text output".
But not: all cases that need full-featured XML output PLUS all cases
that need full-featured JSON output PLUS all cases that need full-
featured CSV output PLUS all cases that need full-featured YAML 
output, etc.


If you want to create a full-featured XML writer that's fine, it's 
just something different from the AVTextFormat API.


Best regards,
sw




_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Stefano Sabatini]

On date Tuesday 2025-04-29 10:30:55 +0200, Nicolas George wrote:
> Stefano Sabatini (HE12025-04-27):
> > Elaborating on this. ffprobe/textformat is based on a notion of
> > hierarchical tree-like data
> 

> No, this is not true. FFprobe and all its supporting code is based on
> one level of hierarchy. Just one level, not a real hierarchical
> structure.

I don't understand this claim. There is a root, and each section can
have several subsections, so it is a tree in my view, although we set
a maximum depth. Where am I wrong?

> > Considering this, there is probably no need to extend the API to cover
> > each possible format full semantics - this at least it is my view.
> 

> If we add XML-writing code in libavutil, it must be usable for all the
> places we are already producing XML. Otherwise it does not make sense.
>
> > As I wrote, this was not the purpose of the ffprobe formats in the
> > first place. MOV/DASH requires a specific use of an XML encoder. In
> > theory it might be done using the textformat API in the current form,
> > but it would be probably pretty awkward. We might want to factorize a
> > few generic utilities (e.g. escaping) to avoid code duplication
> > though.
> 
> You are going at it backwards.
> 
> The goal is not to cram this text writers API forcefully into libavutil
> and then see if it might serve other purposes, which is what softworkz
> is doing.
> 
> The API must be able to handle all our use cases from the start. Until
> then, it goes back to the design board.
> 
> > It might be if we want to make this a generic tool for library users,
> > and for purposes outside of the scope for which it was designed. But I
> > don't think this should be a real blocker - and we might even keep the
> > API private to enable libav* cross-librariers but not
> > external-libraries usage.
> 
> I can concede making a generic API for external users is not blocking.
> Reluctantly.
> 
> But I will not budge on internal uses: this API must serve all our
> current use cases or it stay outside the libraries.

I agree with softworkz on this. The AVTextFormat functionality is not
about a specific format, it's supposed to be a generic way to
represent a data tree using different formats. Being able to provide
this generic representation is crucial, since we want a single entry
point to represent data in a way which can be parsed in various ways,
given a data schema.

If we want to add support for a specific format encoder (e.g. XML,
JSON), it might be *used* by the AVTextFormat API, not be
*implemented* by the AVTextFormat.

_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Stefano
> Sabatini
> Sent: Sonntag, 4. Mai 2025 17:33
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> On date Tuesday 2025-04-29 10:30:55 +0200, Nicolas George wrote:
> > Stefano Sabatini (HE12025-04-27):
> > > Elaborating on this. ffprobe/textformat is based on a notion of
> > > hierarchical tree-like data
> >
> 
> > No, this is not true. FFprobe and all its supporting code is based on
> > one level of hierarchy. Just one level, not a real hierarchical
> > structure.
> 
> I don't understand this claim. There is a root, and each section can
> have several subsections, so it is a tree in my view, although we set
> a maximum depth.


It even goes slightly beyond being a simple tree representation. 
With the various section flags it is possible to further define the 
relations between various nodes like 1-to-1 or 1-to-many and the extensions
for graph printing even allow to designate a certain field as a unique
identifier or as a field representing a relation (kind of a "foreign key").

This diagram (created by yet-unsubmitted code) shows the FFprobe schema
as an Entity-Relationship diagram with markers indicating the kind of 
relation:

https://softworkz.github.io/ffmpeg_output_apis/ffprobe_schema.html
(the fields per entity are incomplete)


Same for the graph printing schema:

https://softworkz.github.io/ffmpeg_output_apis/graph_schema.html
(fields are complete, relationship identifiers marked with PK and FK)


Background:
https://github.com/softworkz/ffmpeg_output_apis/wiki/Schema-Self-Printing


Best regards,
sw






_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Nicolas George]

Stefano Sabatini (HE12025-05-04):
> I don't understand this claim. There is a root, and each section can
> have several subsections, so it is a tree in my view, although we set
> a maximum depth. Where am I wrong?

Are we looking at the same thing? In ffprobe's output, we have sections
“packets”, “streams”, “format”, etc., and in each section items, but
that does not go deeper.

And in the source code of ffprobe, I see extremely ad-hoc code.

> I agree with softworkz on this. The AVTextFormat functionality is not
> about a specific format, it's supposed to be a generic way to
> represent a data tree using different formats. Being able to provide
> this generic representation is crucial, since we want a single entry
> point to represent data in a way which can be parsed in various ways,
> given a data schema.

Is this API meant to be a generic API for writing structured data, or is
it meant to be totally specific to ffprobe and usable by one other use
case that was designed to behave exactly like ffprobe.

An API that is not generic should not go into libavutil.

An API that cannot serve all, or at least most of, our currently
existing use cases cannot be called generic.

> If we want to add support for a specific format encoder (e.g. XML,
> JSON), it might be *used* by the AVTextFormat API, not be
> *implemented* by the AVTextFormat.

Which is exactly what I told softworkz should start with.

Making this API generic is not an easy task, but it is doable. We should
not settle for an inferior API just because the person who proposed it
wrote the code before designing it properly and now is in a hurry to get
it applied.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Nicolas
> George
> Sent: Montag, 5. Mai 2025 16:32
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> Stefano Sabatini (HE12025-05-04):
> > I don't understand this claim. There is a root, and each section can
> > have several subsections, so it is a tree in my view, although we set
> > a maximum depth. Where am I wrong?
> 
> Are we looking at the same thing? In ffprobe's output, we have sections
> “packets”, “streams”, “format”, etc., and in each section items, but
> that does not go deeper.

Please look at those graphs to see that your assumptions are inaccurate:

https://softworkz.github.io/ffmpeg_output_apis/ffprobe_schema.html 
https://softworkz.github.io/ffmpeg_output_apis/graph_schema.html

One example in plain text:

root
  frames (1)
    frame (n)
      frame_tags (n)
      frame_side_data_list (1)
        frame_side_data (n)
          time_codes (1)
            time_code (n)
          frame_side_data_components (1)
            frame_side_data_component (n)
              frame_side_data_pieces (1)
                frame_side_data_piece (n)    


> Making this API generic is not an easy task, but it is doable. We should
> not settle for an inferior API just because the person who proposed it
> wrote the code before designing it properly and now is in a hurry to get
> it applied.

Like I said in my previous response already: I'm not intending to rush 
things in any way. I don't even have a use case for which the API needs
to be located in libavutil. Even when it remains to live in fftools for a 
year or longer, I'm still good with it.
You'll have plenty of time for suggesting improvements or submitting an 
XML writer the way you want it to be, and - given it would be compatible
with the AVTextFormat API - the API could use yours instead of the current
one.

Doors are open for your suggestions, as long as those will fit in to the 
AVTextFormat API logic or are improving it in a way that the consumption
sites don't need to be rewritten (adjustments are surely acceptable).

Best regards,
sw


_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Stefano Sabatini]

On date Monday 2025-05-05 16:32:08 +0200, Nicolas George wrote:
> Stefano Sabatini (HE12025-05-04):
> > I don't understand this claim. There is a root, and each section can
> > have several subsections, so it is a tree in my view, although we set
> > a maximum depth. Where am I wrong?
> 

> Are we looking at the same thing? In ffprobe's output, we have sections
> “packets”, “streams”, “format”, etc., and in each section items, but
> that does not go deeper.

The -sections option will show the ffprobe data "schema".

$ ffprobe -sections -hide_banner
Sections:
W... = Section is a wrapper (contains other sections, no local entries)
.A.. = Section contains an array of elements of the same type
..V. = Section may contain a variable number of fields with variable keys
...T = Section contain a unique type
FLAGS NAME/UNIQUE_NAME
----
W...  root
.A..      chapters
....          chapter
..V.              tags/chapter_tags
....      format
..V.          tags/format_tags
.A..      frames
....          frame
..V.              tags/frame_tags
.A..              side_data_list/frame_side_data_list
..VT                  side_data/frame_side_data
.A..                      timecodes
....                          timecode
.A..                      components/frame_side_data_components
..VT                          component/frame_side_data_component
.A..                              pieces/frame_side_data_pieces
..VT                                  piece/frame_side_data_piece
.A..              logs
....                  log
....          subtitle
.A..      programs
....          program
..V.              tags/program_tags
.A..              streams/program_streams
....                  stream/program_stream
....                      disposition/program_stream_disposition
..V.                      tags/program_stream_tags
.A..      stream_groups
....          stream_group
..V.              tags/stream_group_tags
....              disposition/stream_group_disposition
.A..              components/stream_group_components
..VT                  component/stream_group_component
.A..                      subcomponents
..VT                          subcomponent
.A..                              pieces/stream_group_pieces
..VT                                  piece/stream_group_piece
.A..                                      subpieces
..VT                                          subpiece
.A..                                              blocks
..VT                                                  block
.A..              streams/stream_group_streams
....                  stream/stream_group_stream
....                      disposition/stream_group_stream_disposition
..V.                      tags/stream_group_stream_tags
.A..      streams
....          stream
....              disposition/stream_disposition
..V.              tags/stream_tags
.A..              side_data_list/stream_side_data_list
..VT                  side_data/stream_side_data
.A..      packets
....          packet
..V.              tags/packet_tags
.A..              side_data_list/packet_side_data_list
..VT                  side_data/packet_side_data
....      error
....      program_version
.A..      library_versions
....          library_version
.A..      pixel_formats
....          pixel_format
....              flags/pixel_format_flags
.A..              components/pixel_format_components
....                  component

So yes, this should be a tree, although we hardcode a maximum depth
and the maximum number of items per section - it might be possible to
remove this limitation with some effort. In particular, we migth
benefit from employing a dictionary implementation.
 
> And in the source code of ffprobe, I see extremely ad-hoc code.

This is expected, since this is application-level logic - and we need
to instruct the code to convert the internal data to the
sections-based representation. The recently tagged AVTextFormat API
should be pretty generic. Note that in fact in AVTextFormat there is
no mention to "ffprobe" concepts (packets, frames, etc.) since that's
part of the data schema.

> > I agree with softworkz on this. The AVTextFormat functionality is not
> > about a specific format, it's supposed to be a generic way to
> > represent a data tree using different formats. Being able to provide
> > this generic representation is crucial, since we want a single entry
> > point to represent data in a way which can be parsed in various ways,
> > given a data schema.
> 
> Is this API meant to be a generic API for writing structured data, or is
> it meant to be totally specific to ffprobe and usable by one other use
> case that was designed to behave exactly like ffprobe.
> 
> An API that is not generic should not go into libavutil.
> 
> An API that cannot serve all, or at least most of, our currently
> existing use cases cannot be called generic.

One of the use cases I have in mind is to support structured data
coming from filters - there are several different approaches currently
employed, all of them somehow underkill. For example, some filters
print to the log using a custom format, making this unsuitable for
parsing; others print to a file, employing a custom format.

Ideally I'd like to have such filters employ the AVTextFormat API (or
whatever we want to call it) so that we can generate outputs in one of
the supported format with the minimum effort. Most of the data coming
out from the filters should be mostly shallow - two or three levels -
so you define the schema, select an output, and finally write the
logic to convert the internal data to the structured output.

So this should be generic enough to support this case - we need to
define a data schema, an output format, and the custom conversion
logic. This model should be generic enough - in fact it is possibly
even more powerful than needed - mostly to support XML - since for
that purpose we could be done with a simple dictionary/JSON
representation.

> > If we want to add support for a specific format encoder (e.g. XML,
> > JSON), it might be *used* by the AVTextFormat API, not be
> > *implemented* by the AVTextFormat.
> 
> Which is exactly what I told softworkz should start with.

What I mean is that we might implement e.g. an XML encoder (such as
av_xml_add_element(), av_json_add_attribute(), etc.) and this might be
used in the codebase wherever the XML format is used - including the
AVTextFormat API, but this is not the scope of such API - it is mostly
about providing means to generate a multi-format representation of a
data tree. Probably the name should reflect this -
AVStructuredFormat/AVTreeFormat API.

On the other hand I'm not convinced we might really benefit from an
XML encoder, given that custom code is trivial while a generic API is
more difficult.

> Making this API generic is not an easy task, but it is doable. We should
> not settle for an inferior API just because the person who proposed it
> wrote the code before designing it properly and now is in a hurry to get
> it applied.

The plan is to use fftools as the staging area, since we don't impact
external/internal interfaces, and possibly let it mature to cover more
use cases without impacting users before making it public. Also we can
move it to libavutil and mark is as private before making it
public. But we need to start somewhere.
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Rémi Denis-Courmont]

Le tiistaina 22. huhtikuuta 2025, 7.20.26 Itä-Euroopan kesäaika softworkz . a 
écrit :
> Hi Stefano, Andreas, Nicolas
> and of course, anybody who's interested in the AVTextFormat APIs,
> 
> 
> let me start by saying that I have no intention to rush the
> publicization of those APIs. I think there's still a way to go.
> But it's also true that when you don't have a clear understanding
> of where you actually want to go, you'll hardly arrive there.
> 
> At the implementation level, I sensed that "you" ("FFmpeg")
> are following some principles which are somewhat contradictive to
> those that I'm usually adhering to (e.g. "parameter validation
> being a responsibility of the call site, crashing otherwise
> being acceptable"). Nonetheless, I'm the one who has to adapt,
> and I'm not going to question that.

How do you validate parameters in C in the first place? Pointers are so 
pervasive (in general, as in FFmpeg), and essentially impossible to validate. 
How do you prevent crashing on invalid pointers?

I feel that what you think you are usually doing is not what you think that 
you are actually usually doing.

It makes sense to validate inputs if you are on a trust boundary and/or 
deserialising data. But that's about the only cases (and it's debatable if 
those aren't even two sides of the same coin).

-- 
ヅニ-クーモン・レミ
Hagalund ny stad, f.d. Finska republik Nylands



_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Rémi
> Denis-Courmont
> Sent: Donnerstag, 24. April 2025 20:34
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> Le tiistaina 22. huhtikuuta 2025, 7.20.26 Itä-Euroopan kesäaika
> softworkz . a
> écrit :
> > Hi Stefano, Andreas, Nicolas
> > and of course, anybody who's interested in the AVTextFormat APIs,
> >
> >
> > let me start by saying that I have no intention to rush the
> > publicization of those APIs. I think there's still a way to go.
> > But it's also true that when you don't have a clear understanding
> > of where you actually want to go, you'll hardly arrive there.
> >
> > At the implementation level, I sensed that "you" ("FFmpeg")
> > are following some principles which are somewhat contradictive to
> > those that I'm usually adhering to (e.g. "parameter validation
> > being a responsibility of the call site, crashing otherwise
> > being acceptable"). Nonetheless, I'm the one who has to adapt,
> > and I'm not going to question that.
> 
> How do you validate parameters in C in the first place? Pointers are
> so
> pervasive (in general, as in FFmpeg), and essentially impossible to
> validate.
> How do you prevent crashing on invalid pointers?
> 
> I feel that what you think you are usually doing is not what you think
> that
> you are actually usually doing.
> 
> It makes sense to validate inputs if you are on a trust boundary
> and/or
> deserialising data. But that's about the only cases (and it's
> debatable if
> those aren't even two sides of the same coin).
> 
> --

Hello Rémi,


I'm not sure what you feel about what I would be thinking I would be
usually doing, but that's quite pointless anyway.

To be clear: I'm not intending to enter a discussion about fundamentals,
just very simple: 

Tell me what I should check for and what not in those four groups of
functions and for those things which should be checked, tell me which 
way (return error, return silently, allow segfault or use an assertion).

Then I'll apply that to all those functions in a uniform and consistent
way without even arguing and the case is closed.

I just don't want to leave it alone like now without clear patterns,
that's all 😊

Thanks,
sw






_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Stefano Sabatini]

On date Friday 2025-04-25 13:16:59 +0000, softworkz . wrote:
[...]
> Tell me what I should check for and what not in those four groups of
> functions and for those things which should be checked, tell me which 
> way (return error, return silently, allow segfault or use an assertion).
> 
> Then I'll apply that to all those functions in a uniform and consistent
> way without even arguing and the case is closed.
> 
> I just don't want to leave it alone like now without clear patterns,
> that's all 😊

I don't really have an answer. Probably it's good to start from the
docs, so that we have a definition of the semantics in advance, for
example stating that a pointer should not be NULL and so on so that at
least we know what is to be considered undefined behavior. As noted by
Nicolas, the pattern is dependant on the function behavior and on
practical ergonomy considerations.

It also would be nice to have a good set of guidelines.

Consider the case of a function which writes to a file:

int avbikeshed_write_color_to_file(const char *filename, const char *color)

It might fail in several ways: filename or color might be NULL,
filename might be missing or be a directory, the user might miss
permissions, the color might not be valid (assuming we want to parse
it).

We might want to assume that filename and color are non-NULL, enabling
the function to crash - for this it migth be good to add an assert and
usually this is the responsibility of the programmer to check for
this. While we want to log feedback and enable the program to handle
the case of invalid file/permissions or invalid color.

The pattern here is: assert/crash for NULL pointers (unless it makes
sense to have a NULL pointer), log and return feedback in case of
invalid input which is validated from within the function.

The other case in the API was:
int avbikeshed_add_color_slice(AVBikeshed *bikeshed, int level)

assuming this can only get a limited number of slices
(MAX_SLICE_COUNT, MAX_SLICE_LEVEL).

This might fail in several ways: bikeshed might be NULL or invalid
(e.g. a pointer to an unrelated structure), level might be invalid
(e.g. negative or >MAX_SLICE_LEVEL) or the bikeshed might contain
already too many slices.

The level might be checked by the programmer, so we might decide to
have an assert. About the count check it is validated from within the
function (since we need to access the bikeshed context) so we want to
provide feedback and fail.

For both of these two examples, doing nothing does not seem a good
idea. That's probably only good if we want to enable idem-potency or
when one of the parameter can be interpreted as a "none" argument.

For example:
   if (color == NULL) {
       return 0;
   }

In this case we should specify the behavior in the documentation,
since that defines what is the undefined behavior and the input
expectactions.
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Stefano Sabatini
> Sent: Sonntag, 27. April 2025 12:42
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> On date Friday 2025-04-25 13:16:59 +0000, softworkz . wrote:
> [...]
> > Tell me what I should check for and what not in those four groups of
> > functions and for those things which should be checked, tell me
> which
> > way (return error, return silently, allow segfault or use an
> assertion).
> >
> > Then I'll apply that to all those functions in a uniform and
> consistent
> > way without even arguing and the case is closed.
> >
> > I just don't want to leave it alone like now without clear patterns,
> > that's all 😊
> 
> I don't really have an answer.

...still by far the best one.


> Probably it's good to start from the
> docs, so that we have a definition of the semantics in advance, for
> example stating that a pointer should not be NULL and so on so that at
> least we know what is to be considered undefined behavior. As noted by
> Nicolas, the pattern is dependant on the function behavior and on
> practical ergonomy considerations.
> 
> It also would be nice to have a good set of guidelines.

Exactly. That's one of the things I would like to work out here.


[..]

> This might fail in several ways: bikeshed might be NULL or invalid
> (e.g. a pointer to an unrelated structure), level might be invalid
> (e.g. negative or >MAX_SLICE_LEVEL) or the bikeshed might contain
> already too many slices.
> 
> The level might be checked by the programmer, so we might decide to
> have an assert. About the count check it is validated from within the
> function (since we need to access the bikeshed context) so we want to
> provide feedback and fail.
> 
> For both of these two examples, doing nothing does not seem a good
> idea. That's probably only good if we want to enable idem-potency or
> when one of the parameter can be interpreted as a "none" argument.
> 
> For example:
>    if (color == NULL) {
>        return 0;
>    }
> 
> In this case we should specify the behavior in the documentation,
> since that defines what is the undefined behavior and the input
> expectactions.

This all makes sense and the practical part is now to apply that kind
of considerations to the individual APIs we have.

Probably it's best when I start by making a suggestion as a starting
point, then we can refine it from there:


1. AVTextFormatter Implementations
==================================

print_section_header(AVTextFormatContext *tctx, const void *data);
print_section_footer(AVTextFormatContext *tctx);
print_integer(AVTextFormatContext *tctx, const char * key, int64_t);
print_string(AVTextFormatContext *tctx, const char *key, const char *value);

Rules

- assert tctx and key
- data and value can be null



2. AVTextWriter Implementations
===============================

writer_w8(AVTextWriterContext *wctx, int b);
writer_put_str(AVTextWriterContext *wctx, const char *str);
writer_vprintf(AVTextWriterContext *wctx, const char *fmt, va_list vl);


Rules

- assert wctx
- str, fmt, vl - ?




3. TextFormat API
=================


avtext_print_section_header(*tctx, const void *data, int section_id)
avtext_print_section_footer(*tctx)
avtext_print_integer(*tctx, const char *key, int64_t val)
avtext_print_integer_flags(*tctx, const char *key, int64_t val, int flags)
avtext_print_unit_int(*tctx, const char *key, int value, const char *unit)
avtext_print_rational(*tctx, const char *key, AVRational q, char sep)
avtext_print_time(*tctx, const char *key, int64_t ts, const AVRational *time_base, int is_duration)
avtext_print_ts(*tctx, const char *key, int64_t ts, int is_duration)
avtext_print_string(*tctx, const char *key, const char *val, int flags)
avtext_print_data(*tctx, const char *key, const uint8_t *data, int size)
avtext_print_data_hash(*tctx, const char *key, const uint8_t *data, int size)
avtext_print_integers(*tctx, const char *key, uint8_t *data, int size,
                      const char *format, int columns, int bytes, int offset_add)


Rules

- assert tctx and key
- how about uint8_t *data, unit and val in ..print_string?



4. TextWriter API
=================

avtextwriter_context_open(AVTextWriterContext **pwctx, const AVTextWriter *writer)
avtextwriter_context_close(AVTextWriterContext **pwctx)
avtextwriter_create_stdout(AVTextWriterContext **pwctx)
avtextwriter_create_avio(AVTextWriterContext **pwctx, AVIOContext *avio_ctx, int close_on_uninit)
avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_filename)
avtextwriter_create_buffer(AVTextWriterContext **pwctx, AVBPrint *buffer)


Rules

- **pwctx: leave unchecked
- writer: return AVERROR(EINVAL)
- avio_ctx: assert
- output_filename: log error and return EINVAL
- buffer: assert ?


5. General
==========

Assertions

Which assert - av_assert0() ?


Public/Private


Looking at AVTextFormatContext - should we start thinking about 
which members we would (at least logically) consider public and
which as non-public?


Thanks,
sw






_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Stefano Sabatini]

On date Sunday 2025-04-27 17:54:21 +0000, softworkz . wrote:
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > Stefano Sabatini
> > Sent: Sonntag, 27. April 2025 12:42
> > To: FFmpeg development discussions and patches <ffmpeg-
> > devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> > 
> > On date Friday 2025-04-25 13:16:59 +0000, softworkz . wrote:
> > [...]
> > > Tell me what I should check for and what not in those four groups of
> > > functions and for those things which should be checked, tell me
> > which
> > > way (return error, return silently, allow segfault or use an
> > assertion).
> > >
> > > Then I'll apply that to all those functions in a uniform and
> > consistent
> > > way without even arguing and the case is closed.
> > >
> > > I just don't want to leave it alone like now without clear patterns,
> > > that's all 😊
> > 
> > I don't really have an answer.
> 
> ...still by far the best one.
> 
> 
> > Probably it's good to start from the
> > docs, so that we have a definition of the semantics in advance, for
> > example stating that a pointer should not be NULL and so on so that at
> > least we know what is to be considered undefined behavior. As noted by
> > Nicolas, the pattern is dependant on the function behavior and on
> > practical ergonomy considerations.
> > 
> > It also would be nice to have a good set of guidelines.
> 
> Exactly. That's one of the things I would like to work out here.
> 
> 
> [..]
> 
> > This might fail in several ways: bikeshed might be NULL or invalid
> > (e.g. a pointer to an unrelated structure), level might be invalid
> > (e.g. negative or >MAX_SLICE_LEVEL) or the bikeshed might contain
> > already too many slices.
> > 
> > The level might be checked by the programmer, so we might decide to
> > have an assert. About the count check it is validated from within the
> > function (since we need to access the bikeshed context) so we want to
> > provide feedback and fail.
> > 
> > For both of these two examples, doing nothing does not seem a good
> > idea. That's probably only good if we want to enable idem-potency or
> > when one of the parameter can be interpreted as a "none" argument.
> > 
> > For example:
> >    if (color == NULL) {
> >        return 0;
> >    }
> > 
> > In this case we should specify the behavior in the documentation,
> > since that defines what is the undefined behavior and the input
> > expectactions.
> 
> This all makes sense and the practical part is now to apply that kind
> of considerations to the individual APIs we have.
> 
> Probably it's best when I start by making a suggestion as a starting
> point, then we can refine it from there:
> 
> 
> 1. AVTextFormatter Implementations
> ==================================
> 
> print_section_header(AVTextFormatContext *tctx, const void *data);
> print_section_footer(AVTextFormatContext *tctx);
> print_integer(AVTextFormatContext *tctx, const char * key, int64_t);
> print_string(AVTextFormatContext *tctx, const char *key, const char *value);
> 

> Rules
> 
> - assert tctx and key

> - data and value can be null

Also: should we return en error in case of invalid nesting level?
This is context dependent so maybe this should be a recoverable error
- my guess is yes although this means complicating usage.

> 2. AVTextWriter Implementations
> ===============================
> 
> writer_w8(AVTextWriterContext *wctx, int b);
> writer_put_str(AVTextWriterContext *wctx, const char *str);

> writer_vprintf(AVTextWriterContext *wctx, const char *fmt, va_list vl);

assuming this is directly used by a programmer, the variadic variant
might also make sense

> 
> 
> Rules
> 
> - assert wctx
> - str, fmt, vl - ?

Can the operation fail? Should we return an error code?
 
> 
> 3. TextFormat API
> =================
> 
> 
> avtext_print_section_header(*tctx, const void *data, int section_id)
> avtext_print_section_footer(*tctx)

> avtext_print_integer(*tctx, const char *key, int64_t val)
> avtext_print_integer_flags(*tctx, const char *key, int64_t val, int flags)

a single variant might do (as we have a single print_string)

> avtext_print_unit_int(*tctx, const char *key, int value, const char *unit)
> avtext_print_rational(*tctx, const char *key, AVRational q, char sep)
> avtext_print_time(*tctx, const char *key, int64_t ts, const AVRational *time_base, int is_duration)
> avtext_print_ts(*tctx, const char *key, int64_t ts, int is_duration)
> avtext_print_string(*tctx, const char *key, const char *val, int flags)
> avtext_print_data(*tctx, const char *key, const uint8_t *data, int size)
> avtext_print_data_hash(*tctx, const char *key, const uint8_t *data, int size)

> avtext_print_integers(*tctx, const char *key, uint8_t *data, int size,
>                       const char *format, int columns, int bytes, int offset_add)

is this really needed? also this seems a complication as it implies
tabular format

> 
> 
> Rules
> 
> - assert tctx and key

> - how about uint8_t *data, unit and val in ..print_string?

what are the current use cases? Can we have empty data/unit/val? Do we
need to support null semantics? I seem to remember we do, let's check.

> 4. TextWriter API
> =================
> 
> avtextwriter_context_open(AVTextWriterContext **pwctx, const AVTextWriter *writer)
> avtextwriter_context_close(AVTextWriterContext **pwctx)
> avtextwriter_create_stdout(AVTextWriterContext **pwctx)
> avtextwriter_create_avio(AVTextWriterContext **pwctx, AVIOContext *avio_ctx, int close_on_uninit)
> avtextwriter_create_file(AVTextWriterContext **pwctx, const char *output_filename)
> avtextwriter_create_buffer(AVTextWriterContext **pwctx, AVBPrint *buffer)
> 
> 
> Rules
> 
> - **pwctx: leave unchecked
> - writer: return AVERROR(EINVAL)
> - avio_ctx: assert

> - output_filename: log error and return EINVAL

or better propagate the failure from open (see libavutil/open_file)

> - buffer: assert ?

unless it makes sense to support an empty buffer?

> 
> 5. General
> ==========
> 
> Assertions
> 

> Which assert - av_assert0() ?

they are once-checks, therefore no performance critical, so yes
  
> Public/Private
> 
> 
> Looking at AVTextFormatContext - should we start thinking about 
> which members we would (at least logically) consider public and
> which as non-public?

From what I know there are no public/non-public fields in FF structs,
but we can extend them with private data/class to be handled in
specialization code if needed.
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Stefano
> Sabatini
> Sent: Dienstag, 29. April 2025 00:27
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> On date Sunday 2025-04-27 17:54:21 +0000, softworkz . wrote:
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Stefano Sabatini
> > > Sent: Sonntag, 27. April 2025 12:42
> > > To: FFmpeg development discussions and patches <ffmpeg-
> > > devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> > >

[..]

> >
> > Probably it's best when I start by making a suggestion as a starting
> > point, then we can refine it from there:
> >
> >
> > 1. AVTextFormatter Implementations
> > ==================================
> >
> > print_section_header(AVTextFormatContext *tctx, const void *data);
> > print_section_footer(AVTextFormatContext *tctx);
> > print_integer(AVTextFormatContext *tctx, const char * key, int64_t);
> > print_string(AVTextFormatContext *tctx, const char *key, const char *value);
> >
> 
> > Rules
> >
> > - assert tctx and key
> 
> > - data and value can be null
> 
> Also: should we return en error in case of invalid nesting level?
> This is context dependent so maybe this should be a recoverable error
> - my guess is yes although this means complicating usage.

This is handled by doing nothing in that case. 
The functions are all void. Changing that and checking the return 
values would probably blow-up the printing code in ffprobe.c to 200%
or 300% (when for all) or still a large amount when only for 
section header and footer.

"Do nothing" has always been a behavior for that case, but it wasn't
implemented consistently.

> 
> > 2. AVTextWriter Implementations
> > ===============================
> >
> > writer_w8(AVTextWriterContext *wctx, int b);
> > writer_put_str(AVTextWriterContext *wctx, const char *str);
> 
> > writer_vprintf(AVTextWriterContext *wctx, const char *fmt, va_list vl);
> 
> assuming this is directly used by a programmer, the variadic variant
> might also make sense

That's one of the big questions, whether this should be public.
I'd rather say no - not for consumption. It's rather public in the 
sense that a lib consumer could create their own implementations
and supply it to av_textformat_open(), then use the other textformat
apis to write output - but not by calling the implementation functions
of AVTextWriter directly.



> >
> > Rules
> >
> > - assert wctx
> > - str, fmt, vl - ?
> 
> Can the operation fail? Should we return an error code?

Our implementation functions return void and so do most of
the upstream functions that the writers are calling.

Same as above, probably not economic to add checks for all invocations.


> > 3. TextFormat API
> > =================
> >
> >
> > avtext_print_section_header(*tctx, const void *data, int section_id)
> > avtext_print_section_footer(*tctx)
> 
> > avtext_print_integer(*tctx, const char *key, int64_t val)
> > avtext_print_integer_flags(*tctx, const char *key, int64_t val, int flags)
> 
> a single variant might do (as we have a single print_string)

Yea, I'll try.


> > avtext_print_unit_int(*tctx, const char *key, int value, const char *unit)
> > avtext_print_rational(*tctx, const char *key, AVRational q, char sep)
> > avtext_print_time(*tctx, const char *key, int64_t ts, const AVRational
> *time_base, int is_duration)
> > avtext_print_ts(*tctx, const char *key, int64_t ts, int is_duration)
> > avtext_print_string(*tctx, const char *key, const char *val, int flags)
> > avtext_print_data(*tctx, const char *key, const uint8_t *data, int size)
> > avtext_print_data_hash(*tctx, const char *key, const uint8_t *data, int
> size)
> 
> > avtext_print_integers(*tctx, const char *key, uint8_t *data, int size,
> >                       const char *format, int columns, int bytes, int
> offset_add)
> 
> is this really needed? also this seems a complication as it implies
> tabular format

That's not my addition. It is used in ffprobe.c for DisplayMatrix.


> > Rules
> >
> > - assert tctx and key
> 
> > - how about uint8_t *data, unit and val in ..print_string?
> 
> what are the current use cases? Can we have empty data/unit/val? Do we
> need to support null semantics? I seem to remember we do, let's check.

Ok

> 
> > 4. TextWriter API
> > =================
> >
> > avtextwriter_context_open(AVTextWriterContext **pwctx, const AVTextWriter
> *writer)
> > avtextwriter_context_close(AVTextWriterContext **pwctx)
> > avtextwriter_create_stdout(AVTextWriterContext **pwctx)
> > avtextwriter_create_avio(AVTextWriterContext **pwctx, AVIOContext *avio_ctx,
> int close_on_uninit)
> > avtextwriter_create_file(AVTextWriterContext **pwctx, const char
> *output_filename)
> > avtextwriter_create_buffer(AVTextWriterContext **pwctx, AVBPrint *buffer)
> >
> >
> > Rules
> >
> > - **pwctx: leave unchecked
> > - writer: return AVERROR(EINVAL)
> > - avio_ctx: assert
> 
> > - output_filename: log error and return EINVAL
> 
> or better propagate the failure from open (see libavutil/open_file)

Both is happening already.


> > - buffer: assert ?
> 
> unless it makes sense to support an empty buffer?

I believe it does not.

> 
> >
> > 5. General
> > ==========
> >
> > Assertions
> >
> 
> > Which assert - av_assert0() ?
> 
> they are once-checks, therefore no performance critical, so yes

Alright, thanks.


> 
> > Public/Private
> >
> >
> > Looking at AVTextFormatContext - should we start thinking about
> > which members we would (at least logically) consider public and
> > which as non-public?
> 
> From what I know there are no public/non-public fields in FF structs,
> but we can extend them with private data/class to be handled in
> specialization code if needed.

I meant that the final result would be using nested structs like it's
done in several places meanwhile.
But for now, I just meant to do something like adding a comment line
saying something like "members below this line are not part of the
API.


Thanks again,
sw



_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
> Sent: Dienstag, 29. April 2025 01:24
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Stefano
> > Sabatini
> > Sent: Dienstag, 29. April 2025 00:27
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> >
> > On date Sunday 2025-04-27 17:54:21 +0000, softworkz . wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Stefano Sabatini
> > > > Sent: Sonntag, 27. April 2025 12:42
> > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> > > >
> 
> [..]


Hello Stefano,

I have five new commits for this:

fftools/textformat: Rename variables wctx to tctx
fftools/textformat: Cleanup unneeded includes
fftools/textformat: Add validation for TextFormat API
fftools/textformat: Add validation for AVTextWriter implementations
fftools/textformat: Add validation for AVTextFormatter implementations

Yet I don't believe it makes sense to squash them once again back into 
commits that you have reviewed already, they are much easier to review
separately.

So, if you would agree, I'd merge the current patchset first (once 
Michael confirms the zlib issue being resolved) and send the new commits
as a new patchset then?

Thanks,
sw



_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Stefano Sabatini]

On date Saturday 2025-05-03 08:55:42 +0000, softworkz . wrote:
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of softworkz .
> > Sent: Dienstag, 29. April 2025 01:24
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
[...]
> Hello Stefano,
> 
> I have five new commits for this:
> 
> fftools/textformat: Rename variables wctx to tctx
> fftools/textformat: Cleanup unneeded includes
> fftools/textformat: Add validation for TextFormat API
> fftools/textformat: Add validation for AVTextWriter implementations
> fftools/textformat: Add validation for AVTextFormatter implementations
> 
> Yet I don't believe it makes sense to squash them once again back into 
> commits that you have reviewed already, they are much easier to review
> separately.
> 
> So, if you would agree, I'd merge the current patchset first (once 
> Michael confirms the zlib issue being resolved) and send the new commits
> as a new patchset then?

Feel free to merge patches which have been already approved or
approved with minor nits - in fact this will simplify the task of
reviewing. Please give some more time to review the other ones not yet
approved.
_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[softworkz .]

> -----Original Message-----
> From: Stefano Sabatini <stefasab@gmail.com>
> Sent: Donnerstag, 8. Mai 2025 01:31
> To: softworkz . <softworkz@hotmail.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> 
> On date Saturday 2025-05-03 08:55:42 +0000, softworkz . wrote:
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> softworkz .
> > > Sent: Dienstag, 29. April 2025 01:24
> > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> [...]
> > Hello Stefano,
> >
> > I have five new commits for this:
> >
> > fftools/textformat: Rename variables wctx to tctx
> > fftools/textformat: Cleanup unneeded includes
> > fftools/textformat: Add validation for TextFormat API
> > fftools/textformat: Add validation for AVTextWriter implementations
> > fftools/textformat: Add validation for AVTextFormatter implementations
> >
> > Yet I don't believe it makes sense to squash them once again back into
> > commits that you have reviewed already, they are much easier to review
> > separately.
> >
> > So, if you would agree, I'd merge the current patchset first (once
> > Michael confirms the zlib issue being resolved) and send the new commits
> > as a new patchset then?
> 
> Feel free to merge patches which have been already approved or
> approved with minor nits - in fact this will simplify the task of
> reviewing. Please give some more time to review the other ones not yet
> approved.

I sent out an e-mail yesterday, asking whether anybody would need more time,
and that I'm planning to apply by the end of the week otherwise.
The set is around for three weeks by now and afaik, at least Andreas 
has reviewed the whole set already.
But if anybody needs more time, I'll surely postpone it.

Thanks
sw







_______________________________________________
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".

Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface

[Stefano Sabatini]

On date Wednesday 2025-05-07 23:42:53 +0000, softworkz . wrote:
> > -----Original Message-----
> > From: Stefano Sabatini <stefasab@gmail.com>
> > Sent: Donnerstag, 8. Mai 2025 01:31
> > To: softworkz . <softworkz@hotmail.com>
> > Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> > 
> > On date Saturday 2025-05-03 08:55:42 +0000, softworkz . wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > softworkz .
> > > > Sent: Dienstag, 29. April 2025 01:24
> > > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [RFC] Shaping the AVTextFormat API Surface
> > [...]
> > > Hello Stefano,
> > >
> > > I have five new commits for this:
> > >
> > > fftools/textformat: Rename variables wctx to tctx
> > > fftools/textformat: Cleanup unneeded includes
> > > fftools/textformat: Add validation for TextFormat API
> > > fftools/textformat: Add validation for AVTextWriter implementations
> > > fftools/textformat: Add validation for AVTextFormatter implementations
> > >
> > > Yet I don't believe it makes sense to squash them once again back into
> > > commits that you have reviewed already, they are much easier to review
> > > separately.
> > >
> > > So, if you would agree, I'd merge the current patchset first (once
> > > Michael confirms the zlib issue being resolved) and send the new commits
> > > as a new patchset then?
> > 
> > Feel free to merge patches which have been already approved or
> > approved with minor nits - in fact this will simplify the task of
> > reviewing. Please give some more time to review the other ones not yet
> > approved.
> 

> I sent out an e-mail yesterday, asking whether anybody would need more time,
> and that I'm planning to apply by the end of the week otherwise.
> The set is around for three weeks by now and afaik, at least Andreas 
> has reviewed the whole set already.
> But if anybody needs more time, I'll surely postpone it.

So I won't block on this, especially given that I don't have much time
to review these days and this is not public API so it can be changed
later with no user impact.

I expressed some concerns about some design choices (e.g. the struct
vs flags) which I'd like to be addressed though to avoid code churns
later.
_______________________________________________
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".