[FFmpeg-devel] [PATCH 1/2] avformat/dashdec: Remove limit on length of id
James Almer
jamrial at gmail.com
Tue Mar 2 17:10:16 EET 2021
On 3/2/2021 11:53 AM, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
> You like this more?
Yes, otherwise as there's no defined upper limit we'll just be
increasing the fixed array every now and then when new manifest files
show up with increasingly verbose ids.
> (It seems to me that the earlier code would have use a NULL pointer for
> %s in av_log if there is no id.)
Since the field is mandatory, chances are it never happened because no
valid manifest would be missing it.
>
> libavformat/dashdec.c | 42 +++++++++++++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 6b43fd6278..947b42f816 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -82,7 +82,7 @@ struct representation {
> AVFormatContext *ctx;
> int stream_index;
>
> - char id[32];
> + char *id;
> char *lang;
> int bandwidth;
Unrelated to this patch, but bandwidth is an unsigned int.
https://github.com/Dash-Industry-Forum/DASH/blob/master/mpdvalidator/schemas/DASH-MPD.xsd#L329
> AVRational framerate;
> @@ -361,6 +361,7 @@ static void free_representation(struct representation *pls)
>
> av_freep(&pls->url_template);
> av_freep(&pls->lang);
> + av_freep(&pls->id);
> av_freep(&pls);
> }
>
> @@ -842,7 +843,7 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> char *val = NULL;
> xmlNodePtr baseurl_nodes[4];
> xmlNodePtr representation_node = node;
> - char *rep_id_val, *rep_bandwidth_val;
> + char *rep_bandwidth_val;
> enum AVMediaType type = AVMEDIA_TYPE_UNKNOWN;
>
> // try get information from representation
> @@ -876,8 +877,14 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> representation_segmenttemplate_node = find_child_node_by_name(representation_node, "SegmentTemplate");
> representation_baseurl_node = find_child_node_by_name(representation_node, "BaseURL");
> representation_segmentlist_node = find_child_node_by_name(representation_node, "SegmentList");
> - rep_id_val = xmlGetProp(representation_node, "id");
> rep_bandwidth_val = xmlGetProp(representation_node, "bandwidth");
> + val = xmlGetProp(representation_node, "id");
> + if (val) {
> + rep->id = av_strdup(val);
> + xmlFree(val);
> + if (!rep->id)
> + goto enomem;
> + }
>
> baseurl_nodes[0] = mpd_baseurl_node;
> baseurl_nodes[1] = period_baseurl_node;
> @@ -886,7 +893,7 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
>
> ret = resolve_content_path(s, url, &c->max_url_size, baseurl_nodes, 4);
> c->max_url_size = aligned(c->max_url_size
> - + (rep_id_val ? strlen(rep_id_val) : 0)
> + + (rep->id ? strlen(rep->id) : 0)
> + (rep_bandwidth_val ? strlen(rep_bandwidth_val) : 0));
> if (ret == AVERROR(ENOMEM) || ret == 0)
> goto free;
> @@ -906,7 +913,9 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> goto enomem;
> }
> c->max_url_size = aligned(c->max_url_size + strlen(val));
> - rep->init_section->url = get_content_url(baseurl_nodes, 4, c->max_url_size, rep_id_val, rep_bandwidth_val, val);
> + rep->init_section->url = get_content_url(baseurl_nodes, 4,
> + c->max_url_size, rep->id,
> + rep_bandwidth_val, val);
> xmlFree(val);
> if (!rep->init_section->url)
> goto enomem;
> @@ -915,7 +924,9 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> val = get_val_from_nodes_tab(fragment_templates_tab, 4, "media");
> if (val) {
> c->max_url_size = aligned(c->max_url_size + strlen(val));
> - rep->url_template = get_content_url(baseurl_nodes, 4, c->max_url_size, rep_id_val, rep_bandwidth_val, val);
> + rep->url_template = get_content_url(baseurl_nodes, 4,
> + c->max_url_size, rep->id,
> + rep_bandwidth_val, val);
> xmlFree(val);
> }
> val = get_val_from_nodes_tab(fragment_templates_tab, 4, "presentationTimeOffset");
> @@ -980,7 +991,8 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> av_free(seg);
> goto free;
> }
> - seg->url = get_content_url(baseurl_nodes, 4, c->max_url_size, rep_id_val, rep_bandwidth_val, NULL);
> + seg->url = get_content_url(baseurl_nodes, 4, c->max_url_size,
> + rep->id, rep_bandwidth_val, NULL);
> if (!seg->url)
> goto enomem;
> seg->size = -1;
> @@ -1014,8 +1026,7 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> fragmenturl_node = xmlFirstElementChild(representation_segmentlist_node);
> while (fragmenturl_node) {
> ret = parse_manifest_segmenturlnode(s, rep, fragmenturl_node,
> - baseurl_nodes,
> - rep_id_val,
> + baseurl_nodes, rep->id,
> rep_bandwidth_val);
> if (ret < 0)
> goto free;
> @@ -1035,15 +1046,14 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> }
> }
> } else {
> - av_log(s, AV_LOG_ERROR, "Unknown format of Representation node id[%s] \n", rep_id_val);
> + av_log(s, AV_LOG_ERROR, "Unknown format of Representation node id '%s' \n",
> + rep->id ? rep->id : "");
> goto free;
> }
>
> if (rep->fragment_duration > 0 && !rep->fragment_timescale)
> rep->fragment_timescale = 1;
> rep->bandwidth = rep_bandwidth_val ? atoi(rep_bandwidth_val) : 0;
> - if (rep_id_val)
> - av_strlcpy(rep->id, rep_id_val, sizeof(rep->id));
> rep->framerate = av_make_q(0, 0);
> if (type == AVMEDIA_TYPE_VIDEO) {
> char *rep_framerate_val = xmlGetProp(representation_node, "frameRate");
> @@ -1070,8 +1080,6 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> goto free;
>
> end:
> - if (rep_id_val)
> - xmlFree(rep_id_val);
> if (rep_bandwidth_val)
> xmlFree(rep_bandwidth_val);
>
> @@ -2129,7 +2137,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[0])
> + if (rep->id)
> av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0);
> }
> for (i = 0; i < c->n_audios; i++) {
> @@ -2138,7 +2146,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[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);
> @@ -2149,7 +2157,7 @@ static int dash_read_header(AVFormatContext *s)
> 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[0])
> + if (rep->id)
> av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0);
nit: You could use AV_DICT_DONT_STRDUP_VAL in all three of these and set
rep->id to NULL afterwards, but it's probably a pointless micro
optimization when parsing a small manifest file.
Should be ok either way.
> if (rep->lang) {
> av_dict_set(&rep->assoc_stream->metadata, "language", rep->lang, 0);
>
More information about the ffmpeg-devel
mailing list