[FFmpeg-devel] [PATCH] Explicit avcodec_decode_video2 documentation about picture allocation
Sun May 2 16:44:14 CEST 2010
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 points to valid memory before calling
Case 2: picture->data 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
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
memcpy(picture->data, decoded_data, size); // Doesn't read data
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.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
More information about the ffmpeg-devel