[FFmpeg-devel,04/10] lavf: fix usages of av_get_codec_tag_string()

---
 libavformat/aiffdec.c     | 8 +++-----
 libavformat/apngdec.c     | 8 ++------
 libavformat/avidec.c      | 8 ++------
 libavformat/matroskadec.c | 7 ++-----
 libavformat/movenc.c      | 6 +-----
 libavformat/mux.c         | 7 +++----
 libavformat/rsd.c         | 7 ++-----
 libavformat/wavdec.c      | 5 ++---
 8 files changed, 17 insertions(+), 39 deletions(-)

On 3/27/2017 4:51 AM, Clément Bœsch wrote:
> ---
>  libavformat/aiffdec.c     | 8 +++-----
>  libavformat/apngdec.c     | 8 ++------
>  libavformat/avidec.c      | 8 ++------
>  libavformat/matroskadec.c | 7 ++-----
>  libavformat/movenc.c      | 6 +-----
>  libavformat/mux.c         | 7 +++----
>  libavformat/rsd.c         | 7 ++-----
>  libavformat/wavdec.c      | 5 ++---
>  8 files changed, 17 insertions(+), 39 deletions(-)
> 
> diff --git a/libavformat/aiffdec.c b/libavformat/aiffdec.c
> index 7dcc85f1ed..81db4a069e 100644
> --- a/libavformat/aiffdec.c
> +++ b/libavformat/aiffdec.c
> @@ -128,11 +128,9 @@ static int get_aiff_header(AVFormatContext *s, int size,
>      } else if (version == AIFF_C_VERSION1) {
>          par->codec_tag = avio_rl32(pb);
>          par->codec_id  = ff_codec_get_id(ff_codec_aiff_tags, par->codec_tag);
> -        if (par->codec_id == AV_CODEC_ID_NONE) {
> -            char tag[32];
> -            av_get_codec_tag_string(tag, sizeof(tag), par->codec_tag);
> -            avpriv_request_sample(s, "unknown or unsupported codec tag: %s", tag);
> -        }
> +        if (par->codec_id == AV_CODEC_ID_NONE)
> +            avpriv_request_sample(s, "unknown or unsupported codec tag: %s",
> +                                  av_4cc2str(par->codec_tag));
>          size -= 4;
>      }
>  
> diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c
> index 75dcf74a0c..2acdac1d25 100644
> --- a/libavformat/apngdec.c
> +++ b/libavformat/apngdec.c
> @@ -404,13 +404,9 @@ static int apng_read_packet(AVFormatContext *s, AVPacket *pkt)
>              return ret;
>          return 0;
>      default:
> -        {
> -        char tag_buf[32];
> -
> -        av_get_codec_tag_string(tag_buf, sizeof(tag_buf), tag);
> -        avpriv_request_sample(s, "In-stream tag=%s (0x%08X) len=%"PRIu32, tag_buf, tag, len);
> +        avpriv_request_sample(s, "In-stream tag=%s (0x%08X) len=%"PRIu32,
> +                              av_4cc2str(tag), tag, len);
>          avio_skip(pb, len + 4);
> -        }
>      }
>  
>      /* Handle the unsupported yet cases */
> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> index 788a3abf3b..31d58052e1 100644
> --- a/libavformat/avidec.c
> +++ b/libavformat/avidec.c
> @@ -811,14 +811,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
>                                                              tag1);
>                      /* If codec is not found yet, try with the mov tags. */
>                      if (!st->codecpar->codec_id) {
> -                        char tag_buf[32];
> -                        av_get_codec_tag_string(tag_buf, sizeof(tag_buf), tag1);
>                          st->codecpar->codec_id =
>                              ff_codec_get_id(ff_codec_movvideo_tags, tag1);
>                          if (st->codecpar->codec_id)
>                             av_log(s, AV_LOG_WARNING,
>                                    "mov tag found in avi (fourcc %s)\n",
> -                                  tag_buf);
> +                                  av_4cc2str(tag1));
>                      }
>                      /* This is needed to get the pict type which is necessary
>                       * for generating correct pts. */
> @@ -992,13 +990,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              }
>          default:
>              if (size > 1000000) {
> -                char tag_buf[32];
> -                av_get_codec_tag_string(tag_buf, sizeof(tag_buf), tag);
>                  av_log(s, AV_LOG_ERROR,
>                         "Something went wrong during header parsing, "
>                         "tag %s has size %u, "
>                         "I will ignore it and try to continue anyway.\n",
> -                       tag_buf, size);
> +                       av_4cc2str(tag), size);
>                  if (s->error_recognition & AV_EF_EXPLODE)
>                      goto fail;
>                  avi->movi_list = avio_tell(pb) - 4;
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 2e3c9bf197..c8d2fd4fc7 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -2212,12 +2212,9 @@ static int matroska_parse_tracks(AVFormatContext *s)
>                  fourcc = MKTAG('S','V','Q','3');
>                  codec_id = ff_codec_get_id(ff_codec_movvideo_tags, fourcc);
>              }
> -            if (codec_id == AV_CODEC_ID_NONE) {
> -                char buf[32];
> -                av_get_codec_tag_string(buf, sizeof(buf), fourcc);
> +            if (codec_id == AV_CODEC_ID_NONE)
>                  av_log(matroska->ctx, AV_LOG_ERROR,
> -                       "mov FourCC not found %s.\n", buf);
> -            }
> +                       "mov FourCC not found %s.\n", av_4cc2str(fourcc));
>              if (track->codec_priv.size >= 86) {
>                  bit_depth = AV_RB16(track->codec_priv.data + 82);
>                  ffio_init_context(&b, track->codec_priv.data,
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 11b26708ae..62b9dce678 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -2278,13 +2278,9 @@ static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
>              hdlr_type = "tmcd";
>              descr = "TimeCodeHandler";
>          } else {
> -            char tag_buf[32];
> -            av_get_codec_tag_string(tag_buf, sizeof(tag_buf),
> -                                    track->par->codec_tag);
> -
>              av_log(s, AV_LOG_WARNING,
>                     "Unknown hldr_type for %s / 0x%04X, writing dummy values\n",
> -                   tag_buf, track->par->codec_tag);
> +                   av_4cc2str(track->par->codec_tag), track->par->codec_tag);
>          }
>          if (track->st) {
>              // hdlr.name is used by some players to identify the content title
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 11b09f1b6e..de70902ef1 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -370,12 +370,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              }
>              if (par->codec_tag) {
>                  if (!validate_codec_tag(s, st)) {
> -                    char tagbuf[32], tagbuf2[32];
> -                    av_get_codec_tag_string(tagbuf, sizeof(tagbuf), par->codec_tag);
> -                    av_get_codec_tag_string(tagbuf2, sizeof(tagbuf2), av_codec_get_tag(s->oformat->codec_tag, par->codec_id));
> +                    const uint32_t otag = av_codec_get_tag(s->oformat->codec_tag, par->codec_id);
>                      av_log(s, AV_LOG_ERROR,
>                             "Tag %s/0x%08x incompatible with output codec id '%d' (%s)\n",
> -                           tagbuf, par->codec_tag, par->codec_id, tagbuf2);
> +                           av_4cc2str(par->codec_tag), par->codec_tag,
> +                           par->codec_id, av_4cc2str(otag));
>                      ret = AVERROR_INVALIDDATA;
>                      goto fail;
>                  }
> diff --git a/libavformat/rsd.c b/libavformat/rsd.c
> index 27a3d73981..19ace46832 100644
> --- a/libavformat/rsd.c
> +++ b/libavformat/rsd.c
> @@ -70,16 +70,13 @@ static int rsd_read_header(AVFormatContext *s)
>      par->codec_tag  = avio_rl32(pb);
>      par->codec_id   = ff_codec_get_id(rsd_tags, par->codec_tag);
>      if (!par->codec_id) {
> -        char tag_buf[32];
> -
> -        av_get_codec_tag_string(tag_buf, sizeof(tag_buf), par->codec_tag);

char *tag_buf = av_4cc2str(par->codec_tag); and keep the two line below
unchanged.

>          for (i=0; i < FF_ARRAY_ELEMS(rsd_unsupported_tags); i++) {
>              if (par->codec_tag == rsd_unsupported_tags[i]) {
> -                avpriv_request_sample(s, "Codec tag: %s", tag_buf);
> +                avpriv_request_sample(s, "Codec tag: %s", av_4cc2str(par->codec_tag));
>                  return AVERROR_PATCHWELCOME;
>              }
>          }
> -        av_log(s, AV_LOG_ERROR, "Unknown codec tag: %s\n", tag_buf);
> +        av_log(s, AV_LOG_ERROR, "Unknown codec tag: %s\n", av_4cc2str(par->codec_tag));
>          return AVERROR_INVALIDDATA;
>      }
>  
> diff --git a/libavformat/wavdec.c b/libavformat/wavdec.c
> index a3cd4ffa06..a3038610e9 100644
> --- a/libavformat/wavdec.c
> +++ b/libavformat/wavdec.c
> @@ -323,7 +323,6 @@ static int wav_read_header(AVFormatContext *s)
>      int64_t size, av_uninit(data_size);
>      int64_t sample_count = 0;
>      int rf64 = 0;
> -    char start_code[32];
>      uint32_t tag;
>      AVIOContext *pb      = s->pb;
>      AVStream *st         = NULL;
> @@ -347,8 +346,8 @@ static int wav_read_header(AVFormatContext *s)
>          rf64 = 1;
>          break;
>      default:
> -        av_get_codec_tag_string(start_code, sizeof(start_code), tag);
> -        av_log(s, AV_LOG_ERROR, "invalid start code %s in RIFF header\n", start_code);
> +        av_log(s, AV_LOG_ERROR, "invalid start code %s in RIFF header\n",
> +               av_4cc2str(tag));
>          return AVERROR_INVALIDDATA;
>      }
>  
>

On Mon, Mar 27, 2017 at 11:35:41AM -0300, James Almer wrote:
[...]
> > -        char tag_buf[32];
> > -
> > -        av_get_codec_tag_string(tag_buf, sizeof(tag_buf), par->codec_tag);
> 
> char *tag_buf = av_4cc2str(par->codec_tag); and keep the two line below
> unchanged.
> 

No, I can't do that, av_4cc2str() creates a temporary anonymous buffer
with a reduced lifetime. It can only live within the function call.

[...]

On 3/27/2017 11:50 AM, Clément Bœsch wrote:
> On Mon, Mar 27, 2017 at 11:35:41AM -0300, James Almer wrote:
> [...]
>>> -        char tag_buf[32];
>>> -
>>> -        av_get_codec_tag_string(tag_buf, sizeof(tag_buf), par->codec_tag);
>>
>> char *tag_buf = av_4cc2str(par->codec_tag); and keep the two line below
>> unchanged.
>>
> 
> No, I can't do that, av_4cc2str() creates a temporary anonymous buffer
> with a reduced lifetime. It can only live within the function call.

Then do

char buf[AV_FOURCC_MAX_STRING_SIZE], *tag_buf = av_fourcc_make_string(buf, par->codec_tag);

av_4cc2str() seems mainly useful as a simplification when you're using
it once to me.
But it's a nit, so lgtm either way.

On Mon, Mar 27, 2017 at 11:59:05AM -0300, James Almer wrote:
> On 3/27/2017 11:50 AM, Clément Bœsch wrote:
> > On Mon, Mar 27, 2017 at 11:35:41AM -0300, James Almer wrote:
> > [...]
> >>> -        char tag_buf[32];
> >>> -
> >>> -        av_get_codec_tag_string(tag_buf, sizeof(tag_buf), par->codec_tag);
> >>
> >> char *tag_buf = av_4cc2str(par->codec_tag); and keep the two line below
> >> unchanged.
> >>
> > 
> > No, I can't do that, av_4cc2str() creates a temporary anonymous buffer
> > with a reduced lifetime. It can only live within the function call.
> 
> Then do
> 
> char buf[AV_FOURCC_MAX_STRING_SIZE], *tag_buf = av_fourcc_make_string(buf, par->codec_tag);
> 
> av_4cc2str() seems mainly useful as a simplification when you're using
> it once to me.
> But it's a nit, so lgtm either way.
> 

as you wish, but note that in any case the call is made only once: it's in
an error path.

Le septidi 7 germinal, an CCXXV, Clement Boesch a écrit :
> No, I can't do that, av_4cc2str() creates a temporary anonymous buffer
> with a reduced lifetime. It can only live within the function call.

I believe you are mistaken:

# it has automatic storage duration associated with the enclosing block.

(6.5.2.5 Compound literals, #6)

It would be inconsistent otherwise: the compound literal would exist in
the call to make_string() but disappear before the call to printf().

Regards,

On Mon, Mar 27, 2017 at 05:07:34PM +0200, Nicolas George wrote:
> Le septidi 7 germinal, an CCXXV, Clement Boesch a écrit :
> > No, I can't do that, av_4cc2str() creates a temporary anonymous buffer
> > with a reduced lifetime. It can only live within the function call.
> 
> I believe you are mistaken:
> 
> # it has automatic storage duration associated with the enclosing block.
> 
> (6.5.2.5 Compound literals, #6)
> 
> It would be inconsistent otherwise: the compound literal would exist in
> the call to make_string() but disappear before the call to printf().
> 

Ah, I stand corrected. Alright, will adjust the code. Thanks.