[FFmpeg-devel] [PATCH 4/5] lavf/dashdec: improve memory handling

[Andreas Rheinhardt]

rcombs:
> - Fixes a couple leaks
> - Adds an av_freep equivalent for libxml buffers
> - Avoids some redundant copying
> ---
>  libavformat/dashdec.c | 44 +++++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/libavformat/dashdec.c b/libavformat/dashdec.c
> index c94ce2caca..378c892b87 100644
> --- a/libavformat/dashdec.c
> +++ b/libavformat/dashdec.c
> @@ -147,9 +147,6 @@ typedef struct DASHContext {
>      uint64_t period_duration;
>      uint64_t period_start;
>  
> -    /* AdaptationSet Attribute */
> -    char *adaptationset_lang;
> -
>      int is_live;
>      AVIOInterruptCB *interrupt_callback;
>      char *allowed_extensions;
> @@ -162,6 +159,15 @@ typedef struct DASHContext {
>  
>  } DASHContext;
>  
> +static void xml_freep(void* arg)
> +{
> +    void *val;
> +
> +    memcpy(&val, arg, sizeof(val));
> +    memcpy(arg, &(void *){ NULL }, sizeof(val));
> +    xmlFree(val);
> +}
> +

libxml2 allows to use custom memory allocation functions. Have you tried
to just use our allocation functions which would allow to use av_freep
directly?

>  static int ishttp(char *url)
>  {
>      const char *proto_name = avio_find_protocol_name(url);
> @@ -362,6 +368,8 @@ static void free_representation(struct representation *pls)
>          avformat_close_input(&pls->ctx);
>      }
>  
> +    xml_freep(&pls->lang);
> +
>      av_freep(&pls->url_template);
>      av_freep(&pls);
>  }
> @@ -878,15 +886,9 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
>              ret = AVERROR(ENOMEM);
>              goto end;
>          }
> -        if (c->adaptationset_lang) {
> -            rep->lang = av_strdup(c->adaptationset_lang);
> -            if (!rep->lang) {
> -                av_log(s, AV_LOG_ERROR, "alloc language memory failure\n");
> -                av_freep(&rep);
> -                ret = AVERROR(ENOMEM);
> -                goto end;
> -            }
> -        }
> +
> +        rep->lang = xmlGetProp(adaptationset_node, "lang");
> +
>          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");
> @@ -965,7 +967,8 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
>                  xmlFree(startnumber_val);
>              }
>              if (adaptationset_supplementalproperty_node) {
> -                if (!av_strcasecmp(xmlGetProp(adaptationset_supplementalproperty_node,"schemeIdUri"), "http://dashif.org/guidelines/last-segment-number")) {
> +                char *schemeIdUri = xmlGetProp(adaptationset_supplementalproperty_node, "schemeIdUri");
> +                if (!av_strcasecmp(schemeIdUri, "http://dashif.org/guidelines/last-segment-number")) {
>                      val = xmlGetProp(adaptationset_supplementalproperty_node,"value");
>                      if (!val) {
>                          av_log(s, AV_LOG_ERROR, "Missing value attribute in adaptationset_supplementalproperty_node\n");
> @@ -974,6 +977,7 @@ static int parse_manifest_representation(AVFormatContext *s, const char *url,
>                          xmlFree(val);
>                      }
>                  }
> +                xmlFree(schemeIdUri);
>              }
>  
>              fragment_timeline_node = find_child_node_by_name(representation_segmenttemplate_node, "SegmentTimeline");
> @@ -1120,13 +1124,10 @@ end:
>  
>  static int parse_manifest_adaptationset_attr(AVFormatContext *s, xmlNodePtr adaptationset_node)
>  {
> -    DASHContext *c = s->priv_data;
> -
>      if (!adaptationset_node) {
>          av_log(s, AV_LOG_WARNING, "Cannot get AdaptationSet\n");
>          return AVERROR(EINVAL);
>      }
> -    c->adaptationset_lang = xmlGetProp(adaptationset_node, "lang");
>  
>      return 0;
>  }
> @@ -1139,7 +1140,6 @@ 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 adaptationset_baseurl_node = NULL;
> @@ -1182,7 +1182,6 @@ static int parse_manifest_adaptationset(AVFormatContext *s, const char *url,
>      }
>  
>  err:
> -    av_freep(&c->adaptationset_lang);
>      return ret;
>  }
>  
> @@ -2157,6 +2156,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, "language", rep->lang, 0);
> +                xml_freep(&rep->lang);
> +            }
>          }
>          for (i = 0; i < c->n_audios; i++) {
>              rep = c->audios[i];
> @@ -2168,7 +2171,7 @@ static int dash_read_header(AVFormatContext *s)
>                  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);
> +                xml_freep(&rep->lang);
>              }
>          }
>          for (i = 0; i < c->n_subtitles; i++) {
> @@ -2179,7 +2182,7 @@ static int dash_read_header(AVFormatContext *s)
>                  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);
> +                xml_freep(&rep->lang);
>              }
>          }
>      }
> @@ -2282,6 +2285,7 @@ static int dash_close(AVFormatContext *s)
>      DASHContext *c = s->priv_data;
>      free_audio_list(c);
>      free_video_list(c);
> +    free_subtitle_list(c);
>      av_dict_free(&c->avio_opts);
>      av_freep(&c->base_url);
>      return 0;
>