[FFmpeg-devel] [PATCH v4 2/2] avformat/dashdec: refine adaptionset attribute members
Nicolas George
george at nsup.org
Sat Mar 28 16:49:23 EET 2020
Steven Liu (12020-03-28):
> all the members is used check all the representation useable.
>
> Signed-off-by: Steven Liu <lq at chinaffmpeg.org>
> ---
> libavformat/dashdec.c | 203 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 199 insertions(+), 4 deletions(-)
Thanks. I find this version easier to read, it has less deleted lines
making noise.
>
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index 271202b0a5..63caa696f8 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -108,6 +108,7 @@ struct representation {
> int64_t cur_seg_offset;
> int64_t cur_seg_size;
> struct fragment *cur_seg;
> + char *lang;
>
> /* Currently active Media Initialization Section */
> struct fragment *init_section;
> @@ -123,6 +124,16 @@ typedef struct DASHContext {
> const AVClass *class;
> char *base_url;
>
> + char *adaptionset_lang;
> + uint64_t adaptionset_minbw;
> + uint64_t adaptionset_maxbw;
> + uint64_t adaptionset_minwidth;
> + uint64_t adaptionset_maxwidth;
> + uint64_t adaptionset_minheight;
> + uint64_t adaptionset_maxheight;
> + AVRational adaptionset_minframerate;
> + AVRational adaptionset_maxframerate;
> +
> int n_videos;
> struct representation **videos;
> int n_audios;
> @@ -156,6 +167,85 @@ typedef struct DASHContext {
>
> } DASHContext;
>
> +static int get_ratio_from_string(AVRational *dst, const char *src)
> +{
> + char *end = NULL;
> + char *end_ptr = NULL;
> + long num, den;
> +
> + num = strtol(src, &end_ptr, 10);
> + if (errno == ERANGE || end_ptr == src)
> + return AVERROR_INVALIDDATA;
If you use errno to check for error, it needs to be initialized to 0
before; otherwise, it could be leftover from a completely different part
of the code.
Also, the list of errors in the standard is not exhaustive: errno could
be something else than ERANGE and still indicate a failure.
> +
> + if (num > INT_MAX)
> + return AVERROR_INVALIDDATA;
This could be merged with the previous test. And I think you need to
test for num < INT_MIN too.
> +
> + if (end_ptr[0] == '\0') {
> + dst->den = 1;
> + dst->num = (int)num;
> + return 0;
> + }
> +
> + if (end_ptr[0] != '/')
> + return AVERROR_INVALIDDATA;
> +
> + end_ptr++;
> + den = strtol(end_ptr, &end, 10);
> + if (errno == ERANGE || end == src)
> + return AVERROR_INVALIDDATA;
> +
> + if (den > INT_MAX)
> + return AVERROR_INVALIDDATA;
Same as above, of course.
> +
> + dst->den = (int)den;
> +
> + return 0;
> +}
> +
> +static int dash_prop_get_uint64(AVFormatContext *s, xmlNodePtr node, uint64_t *dst, const char *key)
> +{
> + char *end_ptr = NULL;
> + char *val = xmlGetProp(node, key);
> + int ret = 0;
> + uint64_t tmpval = 0;
> +
> + if (val) {
> + tmpval = strtoull(val, &end_ptr, 10);
> + if (errno == ERANGE) {
> + av_log(s, AV_LOG_WARNING, "overflow/underflow when get value of %s\n", key);
Same remarks about errno.
> + ret = AVERROR_INVALIDDATA;
> + goto out;
> + }
> + if (end_ptr == val || end_ptr[0] != '\0') {
> + av_log(s, AV_LOG_ERROR, "The %s field value is "
> + "not a valid number: %s\n", key, val);
> + ret = AVERROR_INVALIDDATA;
> + goto out;
> + }
> + *dst = tmpval;
I think, if you want to check all cases, you need to check if tmpval is
greater than UINT64_MAX.
> +out:
> + xmlFree(val);
> + }
> + return ret;
> +}
> +
> +static int dash_get_prop_ratio(AVFormatContext *s, xmlNodePtr node, AVRational *member, const char *key)
> +{
> + char *val = xmlGetProp(node, key);
> + int ret = 0;
> + AVRational rate;
> +
> + if (val) {
> + ret = get_ratio_from_string(&rate, val);
> + if (ret < 0)
> + av_log(s, AV_LOG_WARNING, "Ignoring invalid %s frame rate '%s'\n", key, val);
> + xmlFree(val);
> + }
> + member->num = rate.num;
> + member->den = rate.den;
> + return ret;
> +}
> +
> static int ishttp(char *url)
> {
> const char *proto_name = avio_find_protocol_name(url);
> @@ -872,6 +962,15 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> ret = AVERROR(ENOMEM);
> goto end;
> }
> + if (c->adaptionset_lang) {
> + rep->lang = av_strdup(c->adaptionset_lang);
> + if (!rep->lang) {
> + av_log(s, AV_LOG_ERROR, "alloc language memory failure\n");
> + av_freep(&rep);
> + ret = AVERROR(ENOMEM);
> + goto end;
> + }
> + }
> rep->parent = s;
> representation_segmenttemplate_node = find_child_node_by_name(representation_node, "SegmentTemplate");
> representation_baseurl_node = find_child_node_by_name(representation_node, "BaseURL");
> @@ -1057,15 +1156,55 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> }
>
> if (rep) {
> + uint64_t width = 0;
> + uint64_t height = 0;
> +
> if (rep->fragment_duration > 0 && !rep->fragment_timescale)
> rep->fragment_timescale = 1;
> rep->bandwidth = rep_bandwidth_val ? atoi(rep_bandwidth_val) : 0;
> + if (rep_bandwidth_val && (rep->bandwidth > c->adaptionset_maxbw || rep->bandwidth < c->adaptionset_minbw)) {
> + av_log(s, AV_LOG_WARNING, "The bandwidth of representation %s is incorrect, "
> + "will ignore this representation.\n", rep_id_val);
> + free_representation(rep);
> + goto end;
> + }
> +
> + if (dash_prop_get_uint64(s, representation_node, &width, "width") < 0)
> + av_log(s, AV_LOG_WARNING, "Ignoring the width of representation %s is incorrect.\n", rep_id_val);
> + if (width > c->adaptionset_maxwidth || width < c->adaptionset_minwidth) {
> + av_log(s, AV_LOG_WARNING, "will ignore representation %s, width is %"PRIu64".\n", rep_id_val, width);
> + free_representation(rep);
> + goto end;
> + }
> +
> + if (dash_prop_get_uint64(s, representation_node, &height, "height") < 0)
> + av_log(s, AV_LOG_WARNING, "Ignoring the height of representation %s is incorrect.\n", rep_id_val);
> + if (height > c->adaptionset_maxheight || height < c->adaptionset_minheight) {
> + av_log(s, AV_LOG_WARNING, "will ignore representation %s, height is %"PRIu64".\n", rep_id_val, height);
> + free_representation(rep);
> + goto end;
> + }
> +
> strncpy(rep->id, rep_id_val ? rep_id_val : "", sizeof(rep->id));
> rep->framerate = av_make_q(0, 0);
> if (type == AVMEDIA_TYPE_VIDEO && rep_framerate_val) {
> - ret = av_parse_video_rate(&rep->framerate, rep_framerate_val);
> + ret = get_ratio_from_string(&rep->framerate, rep_framerate_val);
> if (ret < 0)
> - av_log(s, AV_LOG_VERBOSE, "Ignoring invalid frame rate '%s'\n", rep_framerate_val);
> + av_log(s, AV_LOG_WARNING, "Ignoring invalid frame rate '%s'\n", rep_framerate_val);
> + if (c->adaptionset_maxframerate.num > 0) {
> + if (av_cmp_q(rep->framerate, c->adaptionset_maxframerate) == 1) {
> + av_log(s, AV_LOG_WARNING, "will ignore representation %s, framerate is %s.\n", rep_id_val, rep_framerate_val);
> + free_representation(rep);
> + goto end;
> + }
> + }
> + if (c->adaptionset_minframerate.num > 0) {
> + if (av_cmp_q(rep->framerate, c->adaptionset_minframerate) == -1) {
> + av_log(s, AV_LOG_WARNING, "will ignore representation %s, framerate is %s.\n", rep_id_val, rep_framerate_val);
> + free_representation(rep);
> + goto end;
> + }
> + }
> }
>
> switch (type) {
> @@ -1093,6 +1232,7 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
> subtitle_rep_idx += type == AVMEDIA_TYPE_SUBTITLE;
>
> end:
> +
Stray change.
> if (rep_id_val)
> xmlFree(rep_id_val);
> if (rep_bandwidth_val)
> @@ -1103,6 +1243,45 @@ end:
> return ret;
> }
>
> +static int parse_manifest_adaptationset_attr(AVFormatContext *s, xmlNodePtr adaptionset_node)
> +{
> + int ret = 0;
> + DASHContext *c = s->priv_data;
> +
> + if (!adaptionset_node) {
> + av_log(s, AV_LOG_WARNING, "Cannot get AdaptionSet\n");
> + return AVERROR(EINVAL);
> + }
> +
> + c->adaptionset_lang = xmlGetProp(adaptionset_node, "lang");
> +
> + if (dash_prop_get_uint64(s, adaptionset_node, &c->adaptionset_minbw, "minBandwidth") < 0)
> + c->adaptionset_minbw = 0;
> +
> + if (dash_prop_get_uint64(s, adaptionset_node, &c->adaptionset_maxbw, "maxBandwidth") < 0)
> + c->adaptionset_maxbw = UINT64_MAX;
> +
> + if (dash_prop_get_uint64(s, adaptionset_node, &c->adaptionset_minwidth, "minWidth") < 0)
> + c->adaptionset_minwidth = 0;
> +
> + if (dash_prop_get_uint64(s, adaptionset_node, &c->adaptionset_maxwidth, "maxWidth") < 0)
> + c->adaptionset_maxwidth = UINT64_MAX;
> +
> + if (dash_prop_get_uint64(s, adaptionset_node, &c->adaptionset_minheight, "minHeight") < 0)
> + c->adaptionset_minheight = 0;
> +
> + if (dash_prop_get_uint64(s, adaptionset_node, &c->adaptionset_maxheight, "maxHeight") < 0)
> + c->adaptionset_maxheight = UINT64_MAX;
> +
> + if (dash_get_prop_ratio(s, adaptionset_node, &c->adaptionset_minframerate, "minFrameRate") < 0)
> + c->adaptionset_minframerate = av_make_q(0, 0);
> +
> + if (dash_get_prop_ratio(s, adaptionset_node, &c->adaptionset_maxframerate, "maxFrameRate") < 0)
> + c->adaptionset_maxframerate = av_make_q(0, 0);
Since you don't really use the return code, make dash_prop_get_uint64()
set the default value itself if it fails.
> +
> + return ret;
> +}
> +
> static int parse_manifest_adaptationset(AVFormatContext *s, const char *url,
> xmlNodePtr adaptionset_node,
> xmlNodePtr mpd_baseurl_node,
> @@ -1111,6 +1290,7 @@ static int parse_manifest_adaptationset(AVFormatContext *s, const char *url,
> xmlNodePtr period_segmentlist_node)
> {
> int ret = 0;
> + DASHContext *c = s->priv_data;
> xmlNodePtr fragment_template_node = NULL;
> xmlNodePtr content_component_node = NULL;
> xmlNodePtr adaptionset_baseurl_node = NULL;
> @@ -1118,6 +1298,10 @@ static int parse_manifest_adaptationset(AVFormatContext *s, const char *url,
> xmlNodePtr adaptionset_supplementalproperty_node = NULL;
> xmlNodePtr node = NULL;
>
> + ret = parse_manifest_adaptationset_attr(s, adaptionset_node);
> + if (ret < 0)
> + return ret;
> +
> node = xmlFirstElementChild(adaptionset_node);
> while (node) {
> if (!av_strcasecmp(node->name, (const char *)"SegmentTemplate")) {
> @@ -1143,12 +1327,15 @@ static int parse_manifest_adaptationset(AVFormatContext *s, const char *url,
> adaptionset_segmentlist_node,
> adaptionset_supplementalproperty_node);
> if (ret < 0) {
> - return ret;
> + goto end;
> }
> }
> node = xmlNextElementSibling(node);
> }
> - return 0;
> +
> +end:
> + av_freep(&c->adaptionset_lang);
> + return ret;
> }
>
> static int parse_programinformation(AVFormatContext *s, xmlNodePtr node)
> @@ -2121,6 +2308,10 @@ static int dash_read_header(AVFormatContext *s)
> av_dict_set_int(&rep->assoc_stream->metadata, "variant_bitrate", rep->bandwidth, 0);
> if (rep->id[0])
> av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0);
> + if (rep->lang) {
> + av_dict_set(&rep->assoc_stream->metadata, "lang", rep->lang, 0);
> + av_freep(&rep->lang);
> + }
> }
> for (i = 0; i < c->n_subtitles; i++) {
> rep = c->subtitles[i];
> @@ -2128,6 +2319,10 @@ static int dash_read_header(AVFormatContext *s)
> rep->assoc_stream = s->streams[rep->stream_index];
> if (rep->id[0])
> av_dict_set(&rep->assoc_stream->metadata, "id", rep->id, 0);
> + if (rep->lang) {
> + av_dict_set(&rep->assoc_stream->metadata, "lang", rep->lang, 0);
> + av_freep(&rep->lang);
> + }
> }
> }
>
Regards,
--
Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200328/ec8c3939/attachment.sig>
More information about the ffmpeg-devel
mailing list