Re: [FFmpeg-devel] [PATCH v4 02/11] fftools/textformat: Quality improvements

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

> -----Original Message-----
> From: Stefano Sabatini <stefasab@gmail.com>
> Sent: Montag, 21. April 2025 19:17
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Cc: softworkz <softworkz@hotmail.com>
> Subject: Re: [FFmpeg-devel] [PATCH v4 02/11] fftools/textformat:
> Quality improvements
> 
> On date Sunday 2025-04-20 22:59:05 +0000, softworkz wrote:
> > From: softworkz <softworkz@hotmail.com>
> >
> 
> Nazigrammar commit log nit: use verb to specify the change action,
> like in: apply quality improvements

Sure.

> 
> Also probably want to have some more details below the headline,
> like:
> 
> Perform multiple improvements to increase code robustness.
> In particular:
> * favor unsigned counters for loops
> * add missing checks
> * avoid possibly leaks
> * move variable declarations to inner scopes when feasible
> * provide explicit type-casting when needed

Yea, that's better. Many thanks for writing it out!


> 
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >  fftools/textformat/avtextformat.c | 111 +++++++++++++++++++--------
> ---
> >  fftools/textformat/avtextformat.h |   6 +-
> >  fftools/textformat/tf_default.c   |   8 ++-
> >  fftools/textformat/tf_ini.c       |   2 +-
> >  fftools/textformat/tf_json.c      |   8 ++-
> >  fftools/textformat/tf_xml.c       |   3 -
> >  fftools/textformat/tw_avio.c      |   9 ++-
> >  7 files changed, 93 insertions(+), 54 deletions(-)
> >
> > diff --git a/fftools/textformat/avtextformat.c
> b/fftools/textformat/avtextformat.c
> > index edbcd0b342..893b11298e 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]);
> >  }
> >
> > @@ -141,7 +140,10 @@ int avtext_context_open(AVTextFormatContext
> **ptctx,
> >      AVTextFormatContext *tctx;
> >      int i, ret = 0;
> >
> > -    if (!(tctx = av_mallocz(sizeof(AVTextFormatContext)))) {
> > +    if (!ptctx || !formatter)
> > +        return AVERROR(EINVAL);
> > +
> 
> > +    if (!((tctx = av_mallocz(sizeof(AVTextFormatContext))))) {
> 
> Sorry to nitpick and if I miss the past discussion, why the added
> parentheses?

https://clang.llvm.org/extra/clang-tidy/checks/bugprone/assignment-in-if-condition.html


> >          ret = AVERROR(ENOMEM);
> >          goto fail;
> >      }
> > @@ -213,25 +215,26 @@ int avtext_context_open(AVTextFormatContext
> **ptctx,
> >                      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),
> >                      av_log(tctx, AV_LOG_ERROR,
> >                             "Invalid UTF8 sequence %s found in
> string validation replace '%s'\n",
> >                             bp.str, tctx-
> >string_validation_replacement);
> > -                return ret;
> > +                av_bprint_finalize(&bp, NULL);
> > +                goto fail;
> >              }
> >          }
> >      }
> > @@ -259,6 +262,9 @@ static const char unit_bit_per_second_str[] =
> "bit/s";
> >
> >  void avtext_print_section_header(AVTextFormatContext *tctx, const
> void *data, int section_id)
> >  {
> > +    if (section_id < 0 || section_id >= tctx->nb_sections)
> > +        return;
> > +
> >      tctx->level++;
> >      av_assert0(tctx->level < SECTION_MAX_NB_LEVELS);
> >
> > @@ -272,6 +278,9 @@ void
> avtext_print_section_header(AVTextFormatContext *tctx, const void
> *data, in
> >
> >  void avtext_print_section_footer(AVTextFormatContext *tctx)
> >  {
> > +    if (tctx->level < 0 || tctx->level >= SECTION_MAX_NB_LEVELS)
> > +        return;
> > +
> >      int section_id = tctx->section[tctx->level]->id;
> >      int parent_section_id = tctx->level
> >          ? tctx->section[tctx->level - 1]->id
> > @@ -289,7 +298,12 @@ void
> avtext_print_section_footer(AVTextFormatContext *tctx)
> >
> >  void avtext_print_integer(AVTextFormatContext *tctx, const char
> *key, int64_t val)
> >  {
> > -    const struct AVTextFormatSection *section = tctx->section[tctx-
> >level];
> > +    const AVTextFormatSection *section;
> > +
> 
> > +    if (!key || tctx->level < 0 || tctx->level >=
> SECTION_MAX_NB_LEVELS)
> > +        return;
> 
> possibly unrelated: maybe we should add an explicit warning or even an
> assert?

I agree, this is actually something that must not happen, not something
that should be silently ignored.


> 
> > +
> > +    section = tctx->section[tctx->level];
> >
> >      if (section->show_all_entries || av_dict_get(section-
> >entries_to_show, key, NULL, 0)) {
> >          tctx->formatter->print_integer(tctx, key, val);
> > @@ -299,24 +313,25 @@ void avtext_print_integer(AVTextFormatContext
> *tctx, const char *key, int64_t va
> >
> 
> >  static inline int validate_string(AVTextFormatContext *tctx, char
> **dstp, const char *src)
> >  {
> > -    const uint8_t *p, *endp;
> > +    const uint8_t *p, *endp, *srcp = (const uint8_t *)src;
> >      AVBPrint dstbuf;
> > +    AVBPrint bp;
> >      int invalid_chars_nb = 0, ret = 0;
> >
> > +    *dstp = NULL;
> >      av_bprint_init(&dstbuf, 0, AV_BPRINT_SIZE_UNLIMITED);
> > +    av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> >
> > -    endp = src + strlen(src);
> > -    for (p = src; *p;) {
> > -        uint32_t code;
> > +    endp = srcp + strlen(src);
> > +    for (p = srcp; *p;) {
> > +        int32_t code;
> >          int invalid = 0;
> >          const uint8_t *p0 = p;
> >
> >          if (av_utf8_decode(&code, &p, endp, tctx-
> >string_validation_utf8_flags) < 0) {
> > -            AVBPrint bp;
> > -            av_bprint_init(&bp, 0, AV_BPRINT_SIZE_AUTOMATIC);
> > -            bprint_bytes(&bp, p0, p-p0);
> > -            av_log(tctx, AV_LOG_DEBUG,
> > -                   "Invalid UTF-8 sequence %s found in string
> '%s'\n", bp.str, src);
> > +            av_bprint_clear(&bp);
> > +            bprint_bytes(&bp, p0, p - p0);
> > +            av_log(tctx, AV_LOG_DEBUG, "Invalid UTF-8 sequence %s
> found in string '%s'\n", bp.str, src);
> >              invalid = 1;
> >          }
> >
> > @@ -336,7 +351,7 @@ static inline int
> validate_string(AVTextFormatContext *tctx, char **dstp, const
> >          }
> >
> >          if (!invalid || tctx->string_validation ==
> AV_TEXTFORMAT_STRING_VALIDATION_IGNORE)
> > -            av_bprint_append_data(&dstbuf, p0, p-p0);
> > +            av_bprint_append_data(&dstbuf, (const char *)p0, p -
> p0);
> >      }
> >
> >      if (invalid_chars_nb && tctx->string_validation ==
> AV_TEXTFORMAT_STRING_VALIDATION_REPLACE)
> > @@ -346,6 +361,7 @@ static inline int
> validate_string(AVTextFormatContext *tctx, char **dstp, const
> >
> >  end:
> >      av_bprint_finalize(&dstbuf, dstp);
> > +    av_bprint_finalize(&bp, NULL);
> >      return ret;
> >  }
> 
> Please split this to a dedicated commit, we want to have a
> justification for this one in the commit log.

Ok, will do.



> > @@ -358,17 +374,18 @@ struct unit_value {
> >      const char *unit;
> >  };
> >
> > -static char *value_string(AVTextFormatContext *tctx, char *buf, int
> buf_size, struct unit_value uv)
> > +static char *value_string(const AVTextFormatContext *tctx, char
> *buf, int buf_size, struct unit_value uv)
> >  {
> >      double vald;
> > -    int64_t vali;
> > +    int64_t vali = 0;
> >      int show_float = 0;
> >
> >      if (uv.unit == unit_second_str) {
> >          vald = uv.val.d;
> >          show_float = 1;
> >      } else {
> > -        vald = vali = uv.val.i;
> > +        vald = (double)uv.val.i;
> > +        vali = uv.val.i;
> >      }
> >
> >      if (uv.unit == unit_second_str && tctx-
> >use_value_sexagesimal_format) {
> > @@ -387,17 +404,17 @@ static char *value_string(AVTextFormatContext
> *tctx, char *buf, int buf_size, st
> >              int64_t index;
> >
> >              if (uv.unit == unit_byte_str && tctx-
> >use_byte_value_binary_prefix) {
> > -                index = (int64_t) (log2(vald)) / 10;
> > -                index = av_clip(index, 0,
> FF_ARRAY_ELEMS(si_prefixes) - 1);
> > +                index = (int64_t)(log2(vald) / 10);
> > +                index = av_clip64(index, 0,
> FF_ARRAY_ELEMS(si_prefixes) - 1);
> >                  vald /= si_prefixes[index].bin_val;
> >                  prefix_string = si_prefixes[index].bin_str;
> >              } else {
> > -                index = (int64_t) (log10(vald)) / 3;
> > -                index = av_clip(index, 0,
> FF_ARRAY_ELEMS(si_prefixes) - 1);
> > +                index = (int64_t)(log10(vald) / 3);
> > +                index = av_clip64(index, 0,
> FF_ARRAY_ELEMS(si_prefixes) - 1);
> >                  vald /= si_prefixes[index].dec_val;
> >                  prefix_string = si_prefixes[index].dec_str;
> >              }
> > -            vali = vald;
> > +            vali = (int64_t)vald;
> >          }
> >
> >          if (show_float || (tctx->use_value_prefix && vald !=
> (int64_t)vald))
> > @@ -425,9 +442,14 @@ void avtext_print_unit_int(AVTextFormatContext
> *tctx, const char *key, int value
> >
> >  int avtext_print_string(AVTextFormatContext *tctx, const char *key,
> const char *val, int flags)
> >  {
> > -    const struct AVTextFormatSection *section = tctx->section[tctx-
> >level];
> > +    const AVTextFormatSection *section;
> >      int ret = 0;
> >
> 
> > +    if (!key || !val || tctx->level < 0 || tctx->level >=
> SECTION_MAX_NB_LEVELS)
> > +        return AVERROR(EINVAL);
> > +
> > +    section = tctx->section[tctx->level];
> 
> ditto

OK


> >      if (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_NEVER ||
> >          (tctx->show_optional_fields == SHOW_OPTIONAL_FIELDS_AUTO
> >              && (flags & AV_TEXTFORMAT_PRINT_STRING_OPTIONAL)
> > @@ -469,12 +491,11 @@ void avtext_print_rational(AVTextFormatContext
> *tctx, const char *key, AVRationa
> >  void avtext_print_time(AVTextFormatContext *tctx, const char *key,
> >                         int64_t ts, const AVRational *time_base, int
> is_duration)
> >  {
> > -    char buf[128];
> > -
> >      if ((!is_duration && ts == AV_NOPTS_VALUE) || (is_duration &&
> ts == 0)) {
> >          avtext_print_string(tctx, key, "N/A",
> AV_TEXTFORMAT_PRINT_STRING_OPTIONAL);
> >      } else {
> > -        double d = ts * av_q2d(*time_base);
> > +        char buf[128];
> > +        double d = av_q2d(*time_base) * ts;
> >          struct unit_value uv;
> >          uv.val.d = d;
> >          uv.unit = unit_second_str;
> > @@ -495,7 +516,8 @@ void avtext_print_data(AVTextFormatContext
> *tctx, const char *name,
> >                         const uint8_t *data, int size)
> >  {
> >      AVBPrint bp;
> > -    int offset = 0, l, i;
> > +    unsigned offset = 0;
> > +    int l, i;
> >
> >      av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> >      av_bprintf(&bp, "\n");
> > @@ -522,25 +544,29 @@ void avtext_print_data(AVTextFormatContext
> *tctx, const char *name,
> >  void avtext_print_data_hash(AVTextFormatContext *tctx, const char
> *name,
> >                              const uint8_t *data, int size)
> >  {
> > -    char *p, buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 };
> > +    char buf[AV_HASH_MAX_SIZE * 2 + 64] = { 0 };
> > +    int len;
> >
> >      if (!tctx->hash)
> >          return;
> >
> >      av_hash_init(tctx->hash);
> >      av_hash_update(tctx->hash, data, size);
> > -    snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx-
> >hash));
> > -    p = buf + strlen(buf);
> > -    av_hash_final_hex(tctx->hash, p, buf + sizeof(buf) - p);
> > +    len = snprintf(buf, sizeof(buf), "%s:", av_hash_get_name(tctx-
> >hash));
> > +    av_hash_final_hex(tctx->hash, (uint8_t *)&buf[len],
> (int)sizeof(buf) - len);
> >      avtext_print_string(tctx, name, buf, 0);
> >  }
> >
> >  void avtext_print_integers(AVTextFormatContext *tctx, const char
> *name,
> > -                                  uint8_t *data, int size, const
> char *format,
> > -                                  int columns, int bytes, int
> offset_add)
> > +                           uint8_t *data, int size, const char
> *format,
> > +                           int columns, int bytes, int offset_add)
> >  {
> >      AVBPrint bp;
> > -    int offset = 0, l, i;
> > +    unsigned offset = 0;
> > +    int l, i;
> > +
> > +    if (!name || !data || !format || columns <= 0 || bytes <= 0)
> > +        return;
> >
> >      av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
> >      av_bprintf(&bp, "\n");
> > @@ -606,12 +632,15 @@ int
> avtextwriter_context_open(AVTextWriterContext **pwctx, const
> AVTextWriter *w
> >      AVTextWriterContext *wctx;
> >      int ret = 0;
> >
> > -    if (!(wctx = av_mallocz(sizeof(AVTextWriterContext)))) {
> > +    if (!pwctx || !writer)
> > +        return AVERROR(EINVAL);
> > +
> > +    if (!((wctx = av_mallocz(sizeof(AVTextWriterContext))))) {
> >          ret = AVERROR(ENOMEM);
> >          goto fail;
> >      }
> >
> > -    if (!(wctx->priv = av_mallocz(writer->priv_size))) {
> > +    if (writer->priv_size && !((wctx->priv = av_mallocz(writer-
> >priv_size)))) {
> >          ret = AVERROR(ENOMEM);
> >          goto fail;
> >      }
> > diff --git a/fftools/textformat/avtextformat.h
> b/fftools/textformat/avtextformat.h
> > index c598af3450..aea691f351 100644
> > --- a/fftools/textformat/avtextformat.h
> > +++ b/fftools/textformat/avtextformat.h
> > @@ -21,9 +21,7 @@
> >  #ifndef FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
> >  #define FFTOOLS_TEXTFORMAT_AVTEXTFORMAT_H
> >
> > -#include <stddef.h>
> >  #include <stdint.h>
> > -#include "libavutil/attributes.h"
> >  #include "libavutil/dict.h"
> >  #include "libavformat/avio.h"
> >  #include "libavutil/bprint.h"
> > @@ -103,7 +101,7 @@ struct AVTextFormatContext {
> >      unsigned int
> nb_item_type[SECTION_MAX_NB_LEVELS][SECTION_MAX_NB_SECTIONS];
> >
> >      /** section per each level */
> > -    const struct AVTextFormatSection
> *section[SECTION_MAX_NB_LEVELS];
> > +    const AVTextFormatSection *section[SECTION_MAX_NB_LEVELS];
> >      AVBPrint section_pbuf[SECTION_MAX_NB_LEVELS]; ///< generic
> print buffer dedicated to each section,
> >                                                    ///  used by
> various formatters
> >
> > @@ -124,7 +122,7 @@ struct AVTextFormatContext {
> >  #define AV_TEXTFORMAT_PRINT_STRING_VALIDATE 2
> >
> >  int avtext_context_open(AVTextFormatContext **ptctx, const
> AVTextFormatter *formatter, AVTextWriterContext *writer_context, const
> char *args,
> > -                        const struct AVTextFormatSection *sections,
> int nb_sections,
> > +                        const AVTextFormatSection *sections, int
> nb_sections,
> >                          int show_value_unit,
> >                          int use_value_prefix,
> >                          int use_byte_value_binary_prefix,
> > diff --git a/fftools/textformat/tf_default.c
> b/fftools/textformat/tf_default.c
> > index 2c5047eafd..ad97173b0b 100644
> > --- a/fftools/textformat/tf_default.c
> > +++ b/fftools/textformat/tf_default.c
> > @@ -68,9 +68,10 @@ DEFINE_FORMATTER_CLASS(default);
> >  /* lame uppercasing routine, assumes the string is lower case ASCII
> */
> >  static inline char *upcase_string(char *dst, size_t dst_size, const
> char *src)
> >  {
> > -    int i;
> > +    unsigned i;
> > +
> >      for (i = 0; src[i] && i < dst_size - 1; i++)
> > -        dst[i] = av_toupper(src[i]);
> > +        dst[i] = (char)av_toupper(src[i]);
> >      dst[i] = 0;
> >      return dst;
> >  }
> > @@ -106,6 +107,9 @@ static void
> default_print_section_footer(AVTextFormatContext *wctx)
> >      const struct AVTextFormatSection *section = wctx->section[wctx-
> >level];
> >      char buf[32];
> >
> > +    if (!section)
> > +        return;
> > +
> >      if (def->noprint_wrappers || def->nested_section[wctx->level])
> >          return;
> >
> > diff --git a/fftools/textformat/tf_ini.c
> b/fftools/textformat/tf_ini.c
> > index 88add0819a..dd77d0e8bf 100644
> > --- a/fftools/textformat/tf_ini.c
> > +++ b/fftools/textformat/tf_ini.c
> > @@ -91,7 +91,7 @@ static char *ini_escape_str(AVBPrint *dst, const
> char *src)
> >              /* fallthrough */
> >          default:
> >              if ((unsigned char)c < 32)
> > -                av_bprintf(dst, "\\x00%02x", c & 0xff);
> > +                av_bprintf(dst, "\\x00%02x", (unsigned char)c);
> >              else
> >                  av_bprint_chars(dst, c, 1);
> >              break;
> > diff --git a/fftools/textformat/tf_json.c
> b/fftools/textformat/tf_json.c
> > index b61d3740c6..e86cdbf5d9 100644
> > --- a/fftools/textformat/tf_json.c
> > +++ b/fftools/textformat/tf_json.c
> > @@ -80,13 +80,18 @@ static const char *json_escape_str(AVBPrint
> *dst, const char *src, void *log_ctx
> >      static const char json_subst[]  = { '"', '\\',  'b',  'f',
> 'n',  'r',  't', 0 };
> >      const char *p;
> >
> 
> > +    if (!src) {
> > +        av_log(log_ctx, AV_LOG_ERROR, "json_escape_str: NULL source
> string\n");
> 
> nit++: warning?
> 
> About error semantic policy, from a quick grep it looks like FFmpeg
> codebase avoids references to the function name, and only provides a
> simple error description, we might do with:
> 
> Cannot escape NULL string, returning NULL

Alright.



> 
> > +        return NULL;
> > +    }
> > +
> >      for (p = src; *p; p++) {
> >          char *s = strchr(json_escape, *p);
> >          if (s) {
> >              av_bprint_chars(dst, '\\', 1);
> >              av_bprint_chars(dst, json_subst[s - json_escape], 1);
> >          } else if ((unsigned char)*p < 32) {
> > -            av_bprintf(dst, "\\u00%02x", *p & 0xff);
> > +            av_bprintf(dst, "\\u00%02x", (unsigned char)*p);
> >          } else {
> >              av_bprint_chars(dst, *p, 1);
> >          }
> > @@ -105,6 +110,7 @@ static void
> json_print_section_header(AVTextFormatContext *wctx, const void *dat
> >          wctx->section[wctx->level-1] : NULL;
> >
> >      if (wctx->level && wctx->nb_item[wctx->level-1])
> > +    if (wctx->level && wctx->nb_item[wctx->level - 1])
> >          writer_put_str(wctx, ",\n");
> >
> >      if (section->flags & AV_TEXTFORMAT_SECTION_FLAG_IS_WRAPPER) {
> > diff --git a/fftools/textformat/tf_xml.c
> b/fftools/textformat/tf_xml.c
> > index befb39246d..28abfc6400 100644
> > --- a/fftools/textformat/tf_xml.c
> > +++ b/fftools/textformat/tf_xml.c
> > @@ -18,10 +18,7 @@
> >   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> >   */
> >
> > -#include <limits.h>
> > -#include <stdarg.h>
> >  #include <stdint.h>
> > -#include <stdio.h>
> >  #include <string.h>
> >
> >  #include "avtextformat.h"
> > diff --git a/fftools/textformat/tw_avio.c
> b/fftools/textformat/tw_avio.c
> > index 6034f74ec9..d1b494b7b4 100644
> > --- a/fftools/textformat/tw_avio.c
> > +++ b/fftools/textformat/tw_avio.c
> > @@ -53,7 +53,7 @@ static void io_w8(AVTextWriterContext *wctx, int
> b)
> >  static void io_put_str(AVTextWriterContext *wctx, const char *str)
> >  {
> >      IOWriterContext *ctx = wctx->priv;
> > -    avio_write(ctx->avio_context, str, strlen(str));
> > +    avio_write(ctx->avio_context, (const unsigned char *)str,
> (int)strlen(str));
> >  }
> >
> >  static void io_printf(AVTextWriterContext *wctx, const char *fmt,
> ...)
> > @@ -78,10 +78,12 @@ const AVTextWriter avtextwriter_avio = {
> >
> >  int avtextwriter_create_file(AVTextWriterContext **pwctx, const
> char *output_filename)
> >  {
> 
> > +    if (!output_filename || !output_filename[0])
> > +        return AVERROR(EINVAL);
> 
> I'd add a warning to aid debugging.

Right!


> >      IOWriterContext *ctx;
> >      int ret;
> >
> > -
> >      ret = avtextwriter_context_open(pwctx, &avtextwriter_avio);
> >      if (ret < 0)
> >          return ret;
> > @@ -103,6 +105,9 @@ int avtextwriter_create_file(AVTextWriterContext
> **pwctx, const char *output_fil
> >
> >  int avtextwriter_create_avio(AVTextWriterContext **pwctx,
> AVIOContext *avio_ctx, int close_on_uninit)
> >  {
> 
> > +    if (!pwctx || !avio_ctx)
> > +        return AVERROR(EINVAL);
> > +
> 
> maybe warning in this case as well

Makes sense!

> 
> [...]
> 
> Looks good to me otherwise.


Thank you!
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".