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

[Cyril Russo]

Le 02/05/2010 16:52, Reimar D?ffinger a ?crit :
> On Sun, May 02, 2010 at 04:44:14PM +0200, Cyril Russo wrote:
>    
>> Le 02/05/2010 15:55, Reimar D?ffinger a ?crit :
>>      
>>> 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]
>>      
> Look, I don't disagree with your documentation improvements, but
> statements like this make you look quite clueless.
> How in the world do you think memcpy could copy data to location
> _stored in the pointer in data[0]_ without _reading_ data[0]?
>    
It can't. In the same way as storing in the pointed "picture" require 
reading "picture" pointer.
I understand your point through.
But since it wasn't obvious to me at first, and since decode_subtitle 
doesn't work the same (check ffmpeg.c:1592), I maintain it's not 
consistent to the rest of the API.
Adding the doc makes no bad, and can only help users like me without 
such wisdom.

>    
>>> 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.
>>      
> Well, I wanted to know from you if you think it is any good,
> I'm not convinced it is.
> However if nobody else has a comment I'll just apply a random one
> soon.
>    
Maybe a mix with the "don't use avpicture_allocate" would be better.
Anyway, you've got plenty of patch to apply now.
Can you look at the metadata_set2 patch too ?

Best regards,
Cyril