Message ID | 20170327075203.7499-4-u@pkh.me |
---|---|
State | Accepted |
Headers | show |
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.
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); 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; }