[FFmpeg-devel] [PATCH v3 1/7] fftools/textformat: Extract and generalize textformat api from ffprobe.c
Stefano Sabatini
stefasab at gmail.com
Sat Mar 8 20:12:13 EET 2025
On date Saturday 2025-03-08 15:30:28 +0000, Soft Works wrote:
>
>
> > -----Original Message-----
> > From: Stefano Sabatini <stefasab at gmail.com>
> > Sent: Samstag, 8. März 2025 15:37
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > Cc: Soft Works <softworkz-at-hotmail.com at ffmpeg.org>; softworkz
> > <softworkz at hotmail.com>; Andreas Rheinhardt
> > <andreas.rheinhardt at outlook.com>
> > Subject: Re: [FFmpeg-devel] [PATCH v3 1/7] fftools/textformat: Extract
> > and generalize textformat api from ffprobe.c
> >
> > On date Saturday 2025-03-01 10:01:58 +0000, softworkz wrote:
> > [...]
> >
> > > +int avtext_context_open(AVTextFormatContext **ptctx, const
> > AVTextFormatter *formatter, AVTextWriterContext *writer, const char
> > *args,
> > > + const struct AVTextFormatSection *sections,
> > int nb_sections,
> > > + int show_value_unit,
> > > + int use_value_prefix,
> > > + int use_byte_value_binary_prefix,
> > > + int use_value_sexagesimal_format,
> > > + int show_optional_fields,
> > > + char *show_data_hash);
> >
> > writer -> writer_ctx?
> >
> > I'm fine with changing this later to avoid massive rebase edits.
>
> No problem, I realized that it's just a matter of tooling after getting annoyed by these things for years 😊
> (done already)
I'm curious what tooling are you using in this case?
[...]
> Naming:
>
> I think the word context is helpful to indicate what it is - like Codec and Codec-Context there's AVTextFormatter and AVTextFormatContext - imo it is good to understand the relation between the too.
>
> For everything else I don't mind. I had changed the namings myself a number of times but each time it ended with another inconsistency - I was just kind of moving the inconsistency around.
> So, I'll happily rename everything in whatever way is desired, I'd just say that before renaming we should make a full plan in advance which covers the full range.
OK, anyway this can be done in a second step so it's not blocking.
Just to elaborate a bit more:
int avcodec_open2(AVCodecContext *avctx, const AVCodec *codec, AVDictionary **options);
note that in avcodec_open2 does not refer to context, although we
provide both a context and a codec. In general adding that word is
providing no added information.
More information about the ffmpeg-devel
mailing list