[vlc-devel] Video quality patches for vlc (based on 0.9.8a)

[Bryan Moore]

Thanks for your explanation, Derk! We initially just made the number
bigger as a workaround, then changed to i_buffer, which you observe
seems incorrect. Yet we are playing 10 Mbps HD video over UDP and
observed worse frame loss until making this modification. The
'increaseReceive...' must not have been enough at 65536, to receive a
300K I-frame without loss. If you can point us to the preferred place to
increase the buffer, or reduce the reallocation latency so that we do
not lose video frames, that would be terrific!

Bob's detailed response follows the context below:

Regards,
Bryan

Subject: Re: Derk-Jan Hartman on i_buffer vs hardcoded '65536' size
[ Video quality patches for vlc (based on 0.9.8a) ]

On April 10, 2009 6:29 AM Derk-Jan Hartman wrote:
> To: Mailing list for VLC media player developers
> Subject: Re: [vlc-devel] Video quality patches for vlc (based on
0.9.8a)
> 
> On 9 apr 2009, at 20:09, Bryan Moore Airgain wrote:
>> FIX #1: frame buffer size
>>
>> modules/demux/live555.cpp file, function 'SessionsSetup',  
>> assignments to 'tk->' members: use 'i_buffer' in lieu of hard-coded
>> 65536 for initial buffer size.
>>
>>   // Airgain begin - increase video buffer to 'i_buffer' bytes (see  
>> above)
>>   // tk->i_buffer = 65536;
>>   // tk->p_buffer = (uint8_t *)malloc( 65536 );
>>   tk->i_buffer = i_buffer;
>>   tk->p_buffer = (uint8_t *)malloc( i_buffer );
>>   // Airgain end
> 
> This seems incorrect. The i_buffer variable holds the value of the  
> what liveMedia should buffer internally (increaseReceiveBufferTo() ) ,

> it does not represent the size of the buffer (tk->p_buffer) in which  
> the results of liveMedia are passed to VLC.


Bob's (Airgain) detailed response:

What drove this change is the following code segment in 'StreamRead'

     /* grow buffer if it looks like buffer is too small, but don't eat
      * up all the memory on strange streams */
     if( i_truncated_bytes > 0 && tk->i_buffer < 2000000 )
     {

[MY NOTES:  'truncated bytes' > 0 means lost data, and this has to
happen
             in order to drive memory re-allocation.  In nearly all HD
             movies this will happen if the initial buffer size is 64k,
             meaning you will always get distorted video.  Sometimes 
             the lost data will crash ffmpeg, or cause it to hang, or 
             drop the audio channel, or drop the video channel, etc.]

         void *p_tmp;
         msg_Dbg( p_demux, "lost %d bytes", i_truncated_bytes );
         msg_Dbg( p_demux, "increasing buffer size to %d", tk->i_buffer
* 2 );
         tk->i_buffer *= 2;
         p_tmp = realloc( tk->p_buffer, tk->i_buffer );
         if( p_tmp == NULL )
         {
             msg_Warn( p_demux, "realloc failed" );
         }
         else
         {
             tk->p_buffer = (uint8_t*)p_tmp;
         }
     }

So what is happening here, is that without the patch, the buffer that
live555 writes to is INITIALLY 64k bytes in size, which is 
grossly underestimated from all of my observations thus far.  Errors
(resulting in dropped packets and distorted video or audio) 
then drive the above code to re-allocate a bigger buffer until there are
no more errors.

The calculated value assigned to 'i_buffer' (for the purposes of the
patch) is 100,000 for audio, and 2,000,000 for video, which we have
already observed to be excellent for rendering HD video up to 1080p.
Case in point, with the default value of 64kb, if you want to play HD
video, you'll probably lose data with the first major I frame, and
possibly several more times after that, until the memory has been
re-allocated to a sufficiently large size.