[FFmpeg-devel] [PATCH 4/5] ogg: allow streams to update metadata

[Reimar Döffinger]

On 20.10.2013, at 09:13, Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> On 19.10.2013, at 22:52, wm4 <nfxjfg at googlemail.com> wrote:
>> On Sat, 19 Oct 2013 22:43:27 +0200
>> Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:
>> 
>>> On 19.10.2013, at 15:40, Ben Boeckel <mathstuf at gmail.com> wrote:
>>>> 
>>>> +        av_free(os->new_metadata);
>>>> +        os->new_metadata      = NULL;
>>> 
>>> These (appearing in two places) can be merged into a single av_freep.
>>> 
>>>> @@ -639,6 +646,7 @@ static int ogg_read_close(AVFormatContext *s)
>>>>           ogg->streams[i].codec->cleanup(s, i);
>>>>       }
>>>>       av_free(ogg->streams[i].private);
>>>> +        av_free(ogg->streams[i].new_metadata);
>>>>   }
>>>>   av_free(ogg->streams);
>>> 
>>> Not really related to your patch, but I wonder why none of these use av_freep.
>> 
>> Why would they? The whole streams array is freed right after this.
> 
> Someone might have a stray pointer to them around?
> So from my point of view it doesn't change things that much whether it is freed or not, av_freep should still be a (little) bit more robust.

I see that Michael changed it already so I guess the discussion is kind of moot though.