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

[Soft Works <softworkz-at-hotmail.com@ffmpeg.org>]

> -----Original Message-----
> From: Stefano Sabatini <stefasab@gmail.com>
> Sent: Samstag, 8. März 2025 15:01
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: Soft Works <softworkz-at-hotmail.com@ffmpeg.org>; softworkz
> <softworkz@hotmail.com>; Andreas Rheinhardt
> <andreas.rheinhardt@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
_______________________________________________
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".