| Message ID | AM7PR03MB666036B5F795EC130A536DE48FC49@AM7PR03MB6660.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | 831718bbab43fb7fd7c3fb4eb28c8565bc980e6a |
| Headers | show |
| Series | [FFmpeg-devel,1/8] avformat/vorbiscomment: Don't compute strlen twice | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 8/23/2021 10:16 AM, Andreas Rheinhardt wrote: > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/movenc.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index 18410c70fa..acf9d63c52 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -2766,6 +2766,7 @@ static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra > MOVMuxContext *mov = s->priv_data; > const char *hdlr, *descr = NULL, *hdlr_type = NULL; > int64_t pos = avio_tell(pb); > + size_t descr_len; > > hdlr = "dhlr"; > hdlr_type = "url "; > @@ -2829,9 +2830,10 @@ static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra > avio_wb32(pb, 0); /* reserved */ > avio_wb32(pb, 0); /* reserved */ > avio_wb32(pb, 0); /* reserved */ > + descr_len = strlen(descr); > if (!track || track->mode == MODE_MOV) > - avio_w8(pb, strlen(descr)); /* pascal string */ > - avio_write(pb, descr, strlen(descr)); /* handler description */ > + avio_w8(pb, descr_len); /* pascal string */ > + avio_write(pb, descr, descr_len); /* handler description */ > if (track && track->mode != MODE_MOV) > avio_w8(pb, 0); /* c string */ > return update_size(pb, pos); > @@ -3502,21 +3504,22 @@ static int mov_write_itunes_hdlr_tag(AVIOContext *pb, MOVMuxContext *mov, > /* helper function to write a data tag with the specified string as data */ > static int mov_write_string_data_tag(AVIOContext *pb, const char *data, int lang, int long_style) > { > + size_t data_len = strlen(data); > if (long_style) { > - int size = 16 + strlen(data); > + int size = 16 + data_len; > avio_wb32(pb, size); /* size */ > ffio_wfourcc(pb, "data"); > avio_wb32(pb, 1); > avio_wb32(pb, 0); > - avio_write(pb, data, strlen(data)); > + avio_write(pb, data, data_len); > return size; > } else { > + avio_wb16(pb, data_len); /* string length */ > if (!lang) > lang = ff_mov_iso639_to_lang("und", 1); > - avio_wb16(pb, strlen(data)); /* string length */ > avio_wb16(pb, lang); > - avio_write(pb, data, strlen(data)); > - return strlen(data) + 4; > + avio_write(pb, data, data_len); > + return data_len + 4; > } > } > > @@ -3792,9 +3795,10 @@ static int mov_write_mdta_keys_tag(AVIOContext *pb, MOVMuxContext *mov, > avio_wb32(pb, 0); /* entry count */ > > while (t = av_dict_get(s->metadata, "", t, AV_DICT_IGNORE_SUFFIX)) { > - avio_wb32(pb, strlen(t->key) + 8); > + size_t key_len = strlen(t->key); > + avio_wb32(pb, key_len + 8); > ffio_wfourcc(pb, "mdta"); > - avio_write(pb, t->key, strlen(t->key)); > + avio_write(pb, t->key, key_len); > count += 1; > } > curpos = avio_tell(pb); nit: could just use avio_printf(pb, "%s", string) for all these instead.
James Almer: > On 8/23/2021 10:16 AM, Andreas Rheinhardt wrote: >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavformat/movenc.c | 22 +++++++++++++--------- >> 1 file changed, 13 insertions(+), 9 deletions(-) >> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >> index 18410c70fa..acf9d63c52 100644 >> --- a/libavformat/movenc.c >> +++ b/libavformat/movenc.c >> @@ -2766,6 +2766,7 @@ static int mov_write_hdlr_tag(AVFormatContext >> *s, AVIOContext *pb, MOVTrack *tra >> MOVMuxContext *mov = s->priv_data; >> const char *hdlr, *descr = NULL, *hdlr_type = NULL; >> int64_t pos = avio_tell(pb); >> + size_t descr_len; >> hdlr = "dhlr"; >> hdlr_type = "url "; >> @@ -2829,9 +2830,10 @@ static int mov_write_hdlr_tag(AVFormatContext >> *s, AVIOContext *pb, MOVTrack *tra >> avio_wb32(pb, 0); /* reserved */ >> avio_wb32(pb, 0); /* reserved */ >> avio_wb32(pb, 0); /* reserved */ >> + descr_len = strlen(descr); >> if (!track || track->mode == MODE_MOV) >> - avio_w8(pb, strlen(descr)); /* pascal string */ >> - avio_write(pb, descr, strlen(descr)); /* handler description */ >> + avio_w8(pb, descr_len); /* pascal string */ >> + avio_write(pb, descr, descr_len); /* handler description */ >> if (track && track->mode != MODE_MOV) >> avio_w8(pb, 0); /* c string */ >> return update_size(pb, pos); >> @@ -3502,21 +3504,22 @@ static int >> mov_write_itunes_hdlr_tag(AVIOContext *pb, MOVMuxContext *mov, >> /* helper function to write a data tag with the specified string as >> data */ >> static int mov_write_string_data_tag(AVIOContext *pb, const char >> *data, int lang, int long_style) >> { >> + size_t data_len = strlen(data); >> if (long_style) { >> - int size = 16 + strlen(data); >> + int size = 16 + data_len; >> avio_wb32(pb, size); /* size */ >> ffio_wfourcc(pb, "data"); >> avio_wb32(pb, 1); >> avio_wb32(pb, 0); >> - avio_write(pb, data, strlen(data)); >> + avio_write(pb, data, data_len); >> return size; >> } else { >> + avio_wb16(pb, data_len); /* string length */ >> if (!lang) >> lang = ff_mov_iso639_to_lang("und", 1); >> - avio_wb16(pb, strlen(data)); /* string length */ >> avio_wb16(pb, lang); >> - avio_write(pb, data, strlen(data)); >> - return strlen(data) + 4; >> + avio_write(pb, data, data_len); >> + return data_len + 4; >> } >> } >> @@ -3792,9 +3795,10 @@ static int >> mov_write_mdta_keys_tag(AVIOContext *pb, MOVMuxContext *mov, >> avio_wb32(pb, 0); /* entry count */ >> while (t = av_dict_get(s->metadata, "", t, >> AV_DICT_IGNORE_SUFFIX)) { >> - avio_wb32(pb, strlen(t->key) + 8); >> + size_t key_len = strlen(t->key); >> + avio_wb32(pb, key_len + 8); >> ffio_wfourcc(pb, "mdta"); >> - avio_write(pb, t->key, strlen(t->key)); >> + avio_write(pb, t->key, key_len); >> count += 1; >> } >> curpos = avio_tell(pb); > > nit: could just use avio_printf(pb, "%s", string) for all these instead. This would implicitly write call strlen() again; it would also implicitly use an AVBPrint with its allocations in case these strings were large. - Andreas
lgtm
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 18410c70fa..acf9d63c52 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -2766,6 +2766,7 @@ static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra MOVMuxContext *mov = s->priv_data; const char *hdlr, *descr = NULL, *hdlr_type = NULL; int64_t pos = avio_tell(pb); + size_t descr_len; hdlr = "dhlr"; hdlr_type = "url "; @@ -2829,9 +2830,10 @@ static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra avio_wb32(pb, 0); /* reserved */ avio_wb32(pb, 0); /* reserved */ avio_wb32(pb, 0); /* reserved */ + descr_len = strlen(descr); if (!track || track->mode == MODE_MOV) - avio_w8(pb, strlen(descr)); /* pascal string */ - avio_write(pb, descr, strlen(descr)); /* handler description */ + avio_w8(pb, descr_len); /* pascal string */ + avio_write(pb, descr, descr_len); /* handler description */ if (track && track->mode != MODE_MOV) avio_w8(pb, 0); /* c string */ return update_size(pb, pos); @@ -3502,21 +3504,22 @@ static int mov_write_itunes_hdlr_tag(AVIOContext *pb, MOVMuxContext *mov, /* helper function to write a data tag with the specified string as data */ static int mov_write_string_data_tag(AVIOContext *pb, const char *data, int lang, int long_style) { + size_t data_len = strlen(data); if (long_style) { - int size = 16 + strlen(data); + int size = 16 + data_len; avio_wb32(pb, size); /* size */ ffio_wfourcc(pb, "data"); avio_wb32(pb, 1); avio_wb32(pb, 0); - avio_write(pb, data, strlen(data)); + avio_write(pb, data, data_len); return size; } else { + avio_wb16(pb, data_len); /* string length */ if (!lang) lang = ff_mov_iso639_to_lang("und", 1); - avio_wb16(pb, strlen(data)); /* string length */ avio_wb16(pb, lang); - avio_write(pb, data, strlen(data)); - return strlen(data) + 4; + avio_write(pb, data, data_len); + return data_len + 4; } } @@ -3792,9 +3795,10 @@ static int mov_write_mdta_keys_tag(AVIOContext *pb, MOVMuxContext *mov, avio_wb32(pb, 0); /* entry count */ while (t = av_dict_get(s->metadata, "", t, AV_DICT_IGNORE_SUFFIX)) { - avio_wb32(pb, strlen(t->key) + 8); + size_t key_len = strlen(t->key); + avio_wb32(pb, key_len + 8); ffio_wfourcc(pb, "mdta"); - avio_write(pb, t->key, strlen(t->key)); + avio_write(pb, t->key, key_len); count += 1; } curpos = avio_tell(pb);
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/movenc.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)