[FFmpeg-devel] [RFC] Avoid av_read_frame memory copy in implementation

[Cyril Russo]

Le 28/05/2010 15:54, Michael Niedermayer a ?crit :
> On Fri, May 28, 2010 at 09:13:50AM +0200, Cyril Russo wrote:
>    
>>
>>      
>>>> Currently, I'm using
>>>> threads, and I have to copy packet because if I don't, it crashes while
>>>> decoding.
>>>>
>>>>          
>>> ffplay uses threads, it does not always copy and it doesnt crash
>>>
>>>        
>> This is wrong.
>>      
> this is tireing, you really dont understand the code at all.
>    
Ok, so I don't understand the code at all.
I've stopped on this code in av_dup_packet which is too mentally 
disturbing to understand:
if((unsigned)pkt->size > (unsigned)pkt->size + FF_INPUT_BUFFER_PADDING_SIZE)
     return AVERROR(ENOMEM);


>
>    
>> ffplay "av_dup" each packet (except special "flush
>> packets"). Please check the first 2 lines of packet_queue_put.
>>      
> av_dup_packet() != always copying a packet
> i suggest you reads its c code
>    
I haven't till today. I've read the documentation however which says:
Warning: This is a hack - the packet memory allocation stuff is broken. 
The packet is allocated if it was not really allocated.

The name of the function is misleading then if it doesn't, as usually 
"dup" means duplicate which is alloc + copy (like in strdup).

>
>    
>> Remove that part and it crashes.
>>      
> remove any random line and it will break one way or another
>    
And ?
The remark was, you need to call av_dup_packet systematically, whatever 
that does, if you don't the code crashes.
>
>    
>> Any code not using the packet immediately must do it, since calling again
>> av_read_frame can destroy the previous returned packet's data.
>>
>> Currently it's:
>> 1) av_read_frame allocates/reuse its packet buffer (the documentation says
>> so, and my tests confirms this)
>> 2) av_read_frame demux in the allocated packet
>> 3) return packet to the user code
>>      
>    
>> 4) malloc packet data in user code
>> 5) copy first packet data in the malloced area in user code
>>      
> i cant help you if you always copy packets
>
>
>    
>> What I think would be better:
>> 1) av_read_frame calls user function to get a buffer (instead of reusing
>> it)
>> 2) av_read_frame demux in the allocated buffer
>> 3) return packet to the user code
>>      
> look you dont understand the code, i tried to explain it to you but you
> ignore what i wrote. So we lack a basis for discussion, you first must
> understand how the demuxers and parsers interact before you can make
> suggestions for a design change.
>    
Ok. I won't waste more of your time.