[FFmpeg-cvslog] r13800 - trunk/libavformat/mov.c

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Thu, Jun 19, 2008 at 04:31:10PM -0700, Baptiste Coudurier wrote:
>> Michael Niedermayer wrote:
>>> On Wed, Jun 18, 2008 at 10:43:59AM +0200, Benoit Fouet wrote:
>>>> bcoudurier wrote:
>>>>> Modified: trunk/libavformat/mov.c
>>>>> ==============================================================================
>>>>> --- trunk/libavformat/mov.c	(original)
>>>>> +++ trunk/libavformat/mov.c	Wed Jun 18 10:30:50 2008
>>>>> @@ -1899,6 +1899,7 @@ static int mov_read_close(AVFormatContex
>>>>>          av_freep(&sc->drefs);
>>>>>          if (sc->pb && sc->pb != s->pb)
>>>>>              url_fclose(sc->pb);
>>>>> +        av_freep(&sc);
>>>>>   
>>>> av_free() should be enough if done on sc
>>> &s->streams[i]->priv_data really should be set to NULL if its freed in
>>> a demuxer! And a comment should be added that its not the correct place
>>> to free priv_data, so the code does not get forgotten ...
>>>
>>> The correct place to free priv_data is av_close_input_stream(). For
>>> muxers its freed in av_write_trailer() and not each muxer individually.
>>> Now the more demuxers add such free but not zero hacks the harder it will
>>> be to fix the bug properly because there will be dozends of double free
>>> errors. (that after all is the reason why i did not just add a free()
>>> in av_close_input_stream(), it requires demuxers to be checked that they
>>> do not free it. The more demuxers free it the more work i have to clean
>>> this mess up, so please if you do add such hacks add a 
>>> //FIXME this is totally wrong and should not be here
>>> st->priv_data = NULL
>>>
>> Yes, patch attached. I reviewed most close functions. I hope I didnt
>> miss anything, regression tests pass.
> 
> looks ok
> 

Applied.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA