[FFmpeg-devel] [PATCH v3 1/7] fftools/textformat: Extract and generalize textformat api from ffprobe.c

[Soft Works]

> -----Original Message-----
> From: Stefano Sabatini <stefasab at gmail.com>
> Sent: Samstag, 8. März 2025 19:12
> To: Soft Works <softworkz at hotmail.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>; Soft Works <softworkz-at-hotmail.com at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v3 1/7] fftools/textformat: Extract
> and generalize textformat api from ffprobe.c
> 
> 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?

SmartGit. It's from a small German company, Win/Linux/Mac. 
Window > Show Log Window - this is what I'm always using. The 'Standard window' is a recent addition but it doesn't provide that workflow where you're really feeling in charge.
At the left side bottom, there's a tree with all branches, local and remote with checkboxes. Those checkboxes are magic. All checked branches are mixed together in the log list. In that list when you select two entries, you get the diff between those shown like normal commits, while the two commits can be from anywhere no matter whether local or remote or even disjoint. 
Another key point is the way how it allows to reshape a range of commits in no time. You can select files in past commits and split them off in a second, drag-drop re-order commits, modify past commits, revert changes from a single file in a past commit on top of the HEAD with a single click. So, when you have a range of commits and someone says change A needs to be in commit X and B moved to Y and all these kinds of things, you can get it done even before you're clear what your first CLI command would need to be.
It's well thought out in so many details and allows me to work like I had always thought it should be.
Sorry for the commercial, but such great tool deserves it imo.

> [...]
> 
> > 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.

Okay fine, no need to argue about that bit!