[FFmpeg-devel] [PATCH] Fix segv when decoding gray8

[Michael Niedermayer]

On Thu, Jun 03, 2010 at 02:34:47PM -0700, Baptiste Coudurier wrote:
> On 06/03/2010 02:22 PM, Michael Niedermayer wrote:
>> On Thu, Jun 03, 2010 at 01:57:22PM -0700, Baptiste Coudurier wrote:
>>> On 06/03/2010 01:48 PM, Michael Niedermayer wrote:
>>>> On Thu, Jun 03, 2010 at 12:11:41PM -0700, Baptiste Coudurier wrote:
>>>>> On 06/03/2010 03:36 AM, Michael Niedermayer wrote:
>>>>>> On Wed, Jun 02, 2010 at 06:39:26PM -0700, Baptiste Coudurier wrote:
>>>>>>> Hi guys,
>>>>>>>
>>>>>>> $subject.
>>>>>>>
>>>>>>> frame->data[1] is set by avpicture_fill which is called with buf 
>>>>>>> passed
>>>>>>> from the demuxer.
>>>>>>> However, for gray8 the data stored is only width*height, so is too
>>>>>>> small
>>>>>>> for the palette, and will segv, memcpying.
>>>>>>>
>>>>>>> I'm not sure what is the right fix here, buf is supposed to be const.
>>>>>>
>>>>>> as this case can only arrise from use of deprecated (and buggy) 
>>>>>> palette
>>>>>> passing API, the correct (long term goal) is to change all codecs so
>>>>>> they
>>>>>> finally stop using this highly unpredictable race condition ridden 
>>>>>> api.
>>>>>> until then, your patch may be ok
>>>>>
>>>>> I don't think this is related to the API.
>>>>>
>>>>> You cannot generate the palette for gray8 (according to pixfmt.h)
>>>>> _anyway_
>>>>> since the data buffer is not big enough, since it comes from the 
>>>>> demuxer
>>>>> itself.
>>>>> So either, the picture needs to be copied and the palette generated, or
>>>>> we
>>>>> skip the palette generation.
>>>>
>>>> maybe iam missing something but why cant  data[1] that contains the
>>>> palette
>>>> point to some pal[256] from the context while data[0] would point to the
>>>> buffer that originates from the demuxer?
>>>
>>> Yes, that should be ok assuming there is instructions about only freeing
>>> data[0], this is what avpicture_free does, but it seems not documented
>>> anywhere.
>>
>> maybe iam silly but i dont think the frames returned by the raw decoder 
>> are
>> freed by avpicture_free() but rather by av_free_packet() or in
>> raw_close_decoder()
>
> How is the user supposed to free the AVFrame given by the decoder ?

with default get/release_buffer(), the user does not, the decoder does.
with overridden get/release_buffer() the user decides into what buffer
the decoder decodes and the decoder tells the user once it does not need
the data anymore


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100603/4eefc3e6/attachment.pgp>