[FFmpeg-devel] [PATCH 4/5] lavf/dashdec: improve memory handling
Ridley Combs
rcombs at rcombs.me
Thu Jun 11 20:28:50 EEST 2020
> On Jun 11, 2020, at 10:19, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
>
> 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?
This would work in theory, but it's not library-safe: if any other library (or the main application) also overrode the libxml2 allocation functions, then this could fail (and if some other code relied on that functionality, we could break that). I'd rather not rely on global state in a dependency lib like that if not absolutely necessary.
>
>> 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;
>>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list