[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 15:01
> 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
> 
> Sorry for delayed review, due to a sickness on my side for the past
> three days.

No worries, I think it was pretty quick and this is waiting for long anyway 😊


I've done all the changes except of type "future improvements" and this:


> > +static const AVTextFormatter *registered_formatters[7+1];
> 
> maybe use a const here, also I'd be more happy if we had a more
> dynamic registration system to avoid the hardcoded bits

While trying this, I remembered that I had tried this before already, but it causes trouble with static initialization order. Probably that's the reason why it has been like this before already, I'm not sure?


> I didn't review the formatters code assuming this was copied and
> adapted from the ffprobe.c file.

Yup.

> The rest looks good to me.

Thanks a lot for your review!

(I'll respond to the other ones separately)

sw