[FFmpeg-devel] [PATCH] Explicit avcodec_decode_video2 documentation about picture allocation

[Cyril Russo]

Le 02/05/2010 15:55, Reimar D?ffinger a ?crit :
> On Sun, May 02, 2010 at 03:20:47PM +0200, Cyril Russo wrote:
>    
>>> I didn't claim it to be obvious, however being only output parameter
>>> (the whole AVFrame) means the function will not read from it - with
>>> that promise there's no way it could know it is allocated.
>>>        
>> I'm not sure the code does what you're saying. Since I haven't
>> checked all video decoders, I will consider to be the case.
>> Anyway, it's not consistent with the rest of the api, for example
>> with avcodec_decode_audio where "samples" is not allocated by the
>> function, but marked as output too.
>>      
> I think you're a bit confused there.
> avcodec_decode_audio3 has an argument
> int16_t *samples
> Obviously it can't allocate "samples" (an array of int16_t) itself,
> because it would have no way of returning the pointer, output parameter
> means that *samples is never read but only written to.
>
> avcodec_decode_video2 has an argument
> AVFrame *picture
> Exactly the same applies here:
> The function can't allocate "picture" (an AVFrame) since it could
> not return the pointer, output parameter just means it will not
> read (the original values of) *picture, it will only write to it.
>    

I'm not confused at all. Here are 2 possible cases:
Case 1: picture->data[0] points to valid memory before calling 
avcodec_decode_video
Case 2: picture->data[0] is NULL before calling avcodec_decode_video

Both case aren't incompatible with your explanation as both don't need 
to read anything from picture, since the required buffer size is known 
beforehand (likely coming from avpicture_alloc).

Case 1 means that you must call avpicture_alloc before calling the 
function.
Case 2 means decode_video allocates the parameter.

Since in decode_audio, the basic algorithm is:
1) allocate space for x samples in "samples"
2) call decode_audio with "samples"

By symmetry, I would do the same with decode_video.
This is wrong and it's not specified in the documentation.

The type of the "AVFrame*" or "int16_t*" could be X or Y but the 
functions don't act the same if it's X or Y and this is disturbing.

Anyway, reading the code of people implementing this (google codesearch 
for avcodec_decode_video), it's clear that half of the people allocate 
the frame before calling the decode_video function and they shouldn't 
since they leak memory.

> Not reading from *picture means it can't access the picture->data
> passed in to it, so allocating it beforehand makes no sense.
>    

I disagree. It doesn't have to read picture->data even if it's allocated 
beforehand.
memcpy(picture->data[0], decoded_data, size); // Doesn't read data[0]

Even if it's obvious in your logic, please add a note in the 
documentation stating your logic.

>
> Let me clarify: I meant adding to the function documentation
> "*picture is initialized and allocated by this function"
> No idea if that is any clearer than your explanation.
>    
Ok, attached.

Cheers,
Cyril


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: allocationHelp.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100502/c011f303/attachment.asc>