| Message ID | 20210302145351.514945-2-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | ea3953ad406e96e95c533ef850c6395da6a7dd86 |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avformat/dashdec: Remove limit on length of id | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On 3/2/2021 11:53 AM, Andreas Rheinhardt wrote: > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > Using separate lists for audio, video and subtitles leads to lots of > code duplication in this demuxer; the only place where one gains a bit > is in refresh_manifest (which is already buggy in itself because it > leaks upon error...). Is it really worth it? > > libavformat/dashdec.c | 27 +++++++++++++-------------- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c > index 947b42f816..6f3f28dcc7 100644 > --- a/libavformat/dashdec.c > +++ b/libavformat/dashdec.c > @@ -2039,6 +2039,14 @@ static int copy_init_section(struct representation *rep_dest, struct representat > > static int dash_close(AVFormatContext *s); > > +static void move_metadata(AVStream *st, const char *key, char **value) > +{ > + if (*value) { > + av_dict_set(&st->metadata, key, *value, AV_DICT_DONT_STRDUP_VAL); > + *value = NULL; > + } > +} > + > static int dash_read_header(AVFormatContext *s) > { > DASHContext *c = s->priv_data; > @@ -2137,8 +2145,7 @@ static int dash_read_header(AVFormatContext *s) > rep->assoc_stream = s->streams[rep->stream_index]; > if (rep->bandwidth > 0) > av_dict_set_int(&rep->assoc_stream->metadata, "variant_bitrate", rep->bandwidth, 0); > - if (rep->id) > - av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0); > + move_metadata(rep->assoc_stream, "id", &rep->id); > } > for (i = 0; i < c->n_audios; i++) { > rep = c->audios[i]; > @@ -2146,23 +2153,15 @@ static int dash_read_header(AVFormatContext *s) > rep->assoc_stream = s->streams[rep->stream_index]; > if (rep->bandwidth > 0) > av_dict_set_int(&rep->assoc_stream->metadata, "variant_bitrate", rep->bandwidth, 0); > - if (rep->id) > - av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0); > - if (rep->lang) { > - av_dict_set(&rep->assoc_stream->metadata, "language", rep->lang, 0); > - av_freep(&rep->lang); > - } > + move_metadata(rep->assoc_stream, "id", &rep->id); > + move_metadata(rep->assoc_stream, "language", &rep->lang); > } > for (i = 0; i < c->n_subtitles; i++) { > rep = c->subtitles[i]; > av_program_add_stream_index(s, 0, rep->stream_index); > rep->assoc_stream = s->streams[rep->stream_index]; > - if (rep->id) > - av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0); > - if (rep->lang) { > - av_dict_set(&rep->assoc_stream->metadata, "language", rep->lang, 0); > - av_freep(&rep->lang); > - } > + move_metadata(rep->assoc_stream, "id", &rep->id); Well, forget what i said at the end of my review for the previous patch :p > + move_metadata(rep->assoc_stream, "language", &rep->lang); > } > > return 0; >
diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c index 947b42f816..6f3f28dcc7 100644 --- a/libavformat/dashdec.c +++ b/libavformat/dashdec.c @@ -2039,6 +2039,14 @@ static int copy_init_section(struct representation *rep_dest, struct representat static int dash_close(AVFormatContext *s); +static void move_metadata(AVStream *st, const char *key, char **value) +{ + if (*value) { + av_dict_set(&st->metadata, key, *value, AV_DICT_DONT_STRDUP_VAL); + *value = NULL; + } +} + static int dash_read_header(AVFormatContext *s) { DASHContext *c = s->priv_data; @@ -2137,8 +2145,7 @@ static int dash_read_header(AVFormatContext *s) rep->assoc_stream = s->streams[rep->stream_index]; if (rep->bandwidth > 0) av_dict_set_int(&rep->assoc_stream->metadata, "variant_bitrate", rep->bandwidth, 0); - if (rep->id) - av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0); + move_metadata(rep->assoc_stream, "id", &rep->id); } for (i = 0; i < c->n_audios; i++) { rep = c->audios[i]; @@ -2146,23 +2153,15 @@ static int dash_read_header(AVFormatContext *s) rep->assoc_stream = s->streams[rep->stream_index]; if (rep->bandwidth > 0) av_dict_set_int(&rep->assoc_stream->metadata, "variant_bitrate", rep->bandwidth, 0); - if (rep->id) - av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0); - if (rep->lang) { - av_dict_set(&rep->assoc_stream->metadata, "language", rep->lang, 0); - av_freep(&rep->lang); - } + move_metadata(rep->assoc_stream, "id", &rep->id); + move_metadata(rep->assoc_stream, "language", &rep->lang); } for (i = 0; i < c->n_subtitles; i++) { rep = c->subtitles[i]; av_program_add_stream_index(s, 0, rep->stream_index); rep->assoc_stream = s->streams[rep->stream_index]; - if (rep->id) - av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0); - if (rep->lang) { - av_dict_set(&rep->assoc_stream->metadata, "language", rep->lang, 0); - av_freep(&rep->lang); - } + move_metadata(rep->assoc_stream, "id", &rep->id); + move_metadata(rep->assoc_stream, "language", &rep->lang); } return 0;
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- Using separate lists for audio, video and subtitles leads to lots of code duplication in this demuxer; the only place where one gains a bit is in refresh_manifest (which is already buggy in itself because it leaks upon error...). Is it really worth it? libavformat/dashdec.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)