> -----Original Message-----
> From: Stefano Sabatini <stefa...@gmail.com>
> Sent: Donnerstag, 24. April 2025 00:35
> To: FFmpeg development discussions and patches <ffmpeg-
> de...@ffmpeg.org>
> Cc: softworkz <softwo...@hotmail.com>
> Subject: Re: [FFmpeg-devel] [PATCH v5 02/14] fftools/textformat: Apply
> quality improvements
>
> On date Tuesday 2025-04-22 21:55:31 +0000, softworkz wrote:
> > From: softworkz <softwo...@hotmail.com>
> >
> > Perform multiple improvements to increase code robustness.
> > In particular:
> > - favor unsigned counters for loops
> > - add missing checks
>
> > - avoid possibly leaks
>
> my typo: possible leaks
Fixed!
>
> > - move variable declarations to inner scopes when feasible
> > - provide explicit type-casting when needed
> >
> > Signed-off-by: softworkz <softwo...@hotmail.com>
> > ---
>
> General nit about headline caseing: from the log most commit use
> all lowercase in headline (I personally only use that form and at some
> point everybody was using that).
Looking at the 250 most recent commit messages, about 60% have an uppercase
letter after the first colon.
> > fftools/textformat/avtextformat.c | 85 ++++++++++++++++++++--------
> ---
> > fftools/textformat/avtextformat.h | 6 +--
> > fftools/textformat/tf_default.c | 8 ++-
> > fftools/textformat/tf_ini.c | 2 +-
> > fftools/textformat/tf_json.c | 17 ++++---
> > fftools/textformat/tf_xml.c | 3 --
> > fftools/textformat/tw_avio.c | 11 +++-
> > 7 files changed, 83 insertions(+), 49 deletions(-)
> >
> > diff --git a/fftools/textformat/avtextformat.c
> b/fftools/textformat/avtextformat.c
> > index 74d179c516..1939a1f739 100644
> > --- a/fftools/textformat/avtextformat.c
> > +++ b/fftools/textformat/avtextformat.c
> > @@ -93,9 +93,8 @@ static const AVClass textcontext_class = {
> >
> > static void bprint_bytes(AVBPrint *bp, const uint8_t *ubuf, size_t
> ubuf_size)
> > {
> > - int i;
> > av_bprintf(bp, "0X");
> > - for (i = 0; i < ubuf_size; i++)
> > + for (unsigned i = 0; i < ubuf_size; i++)
> > av_bprintf(bp, "%02X", ubuf[i]);
> > }
> >
> > @@ -137,6 +136,9 @@ int avtext_context_open(AVTextFormatContext
> **ptctx, const AVTextFormatter *form
> > AVTextFormatContext *tctx;
> > int i, ret = 0;
> >
> > + if (!ptctx || !formatter)
> > + return AVERROR(EINVAL);
> > +
> > if (!(tctx = av_mallocz(sizeof(AVTextFormatContext)))) {
> > ret = AVERROR(ENOMEM);
> > goto fail;
> > @@ -209,25 +211,26 @@ int avtext_context_open(AVTextFormatContext
> **ptctx, const AVTextFormatter *form
> > av_log(NULL, AV_LOG_ERROR, " %s", n);
> > av_log(NULL, AV_LOG_ERROR, "\n");
> > }
> > - return ret;
> > + goto fail;
> > }
> >
> > /* validate replace string */
> > {
> > - const uint8_t *p = tctx->string_validation_replacement;
> > - const uint8_t *endp = p + strlen(p);
> > + const uint8_t *p = (uint8_t *)tctx-
> >string_validation_replacement;
> > + const uint8_t *endp = p + strlen((const char *)p);
> > while (*p) {
> > const uint8_t *p0 = p;
> > int32_t code;
> > ret = av_utf8_decode(&code, &p, endp, tctx-
> >string_validation_utf8_flags);
>
> > if (ret < 0) {
> > AVBPrint bp;
> > - av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC);
> > + av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> > bprint_bytes(&bp, p0, p - p0),
>
> Is this really needed? AUTOMATIC should be faster since it will avoid
> dynamic allocation and the need to finalize (although there is no
> practical need for such optimization, this still seems the simplest
> possible path). Besides, an UTF8 sequence cannot be longer than a few
> bytes, so possibly av_utf8_decode cannot decode more than a few bytes.
As replied earlier, AUTOMATIC is not faster or slower than UNLIMITED
as long as the stack-allocated memory is sufficient. Only when it grows
bigger: UNLIMITED will auto-allocate as needed and AUTOMATIC will
truncate and you'd need to check for that.
UNLIMITED gives you a "no-brain-pain-always-fine" behavior at the
small cost of calling avbp_finalize().
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-requ...@ffmpeg.org with subject "unsubscribe".
