[FFmpeg-devel] [PATCH 2/2] Free allocated packet before returning from av_read_frame_internal.

[Jai Menon]

On Sat, Jul 17, 2010 at 5:25 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Thu, Jul 15, 2010 at 10:46:31AM +0530, Jai Menon wrote:
>> On Thu, Jul 15, 2010 at 2:45 AM, Reimar D?ffinger
>> <Reimar.Doeffinger at gmx.de> wrote:
>> > On Thu, Jul 15, 2010 at 02:35:45AM +0530, Jai Menon wrote:
>> >> ---
>> >> ?libavformat/utils.c | ? ?1 +
>> >> ?1 files changed, 1 insertions(+), 0 deletions(-)
>> >>
>> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> >> index 6fa4dff..bbe6a64 100644
>> >> --- a/libavformat/utils.c
>> >> +++ b/libavformat/utils.c
>> >> @@ -1118,6 +1118,7 @@ static int av_read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?goto got_packet;
>> >> ? ? ? ? ? ? ? ? ? ? ?}
>> >> ? ? ? ? ? ? ? ? ?}
>> >> + ? ? ? ? ? ? ? ?av_free_packet(&cur_pkt);
>> >
>> > It is missing from the av_read_frame documentation, but from the read_packet for
>> > the formats:
>> > ? ? ? ? ? ? ? When returning an error, pkt must not have been allocated
>> > ? ? ? ? ? ? ? or must be freed before returning
>> >
>> > You really shouldn't touch cur_pkt when the function failed, it might not even have been
>> > fully/properly initialized.
>>
>> In this case, the packet is allocated and data read properly, but
>> dv_produce_packet errors out. I guess then a free_packet call should
>> be made like attached?
>>
>> --
>> Jai Menon
>
>> ?avidec.c | ? ?2 ++
>> ?1 file changed, 2 insertions(+)
>> 7281acbe1f9f8fcaafb9e8e164627f200f2cdeed ?avidec.diff
>> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
>> index 058acd5..05cfd98 100644
>> --- a/libavformat/avidec.c
>> +++ b/libavformat/avidec.c
>> @@ -808,6 +808,8 @@ resync:
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pkt->data, pkt->size);
>> ? ? ? ? ? ? ?pkt->destruct = dstr;
>> ? ? ? ? ? ? ?pkt->flags |= AV_PKT_FLAG_KEY;
>> + ? ? ? ? ? ?if (size < 0)
>> + ? ? ? ? ? ? ? ?av_free_packet(pkt);
>
> ok or you could add that to dv_produce_packet()

went with this because the rawdv demuxer calls dv_produce_packet with
buffers on the stack so we wouldnt need it there anyway.
applied.

-- 
Jai Menon