[vlc-devel] [PATCH] Clean the played waveout frames in Play and Stop

[Rémi Denis-Courmont]

On Sat, 29 Dec 2012 16:38:50 +0100, Denis Charmet <typx at dinauz.org> wrote:
> @@ -409,12 +418,16 @@ static void Probe( audio_output_t * p_aout, const
> audio_sample_format_t *fmt 
>  
*****************************************************************************/
>  static void Play( audio_output_t *p_aout, block_t *block )
>  {
> -    WAVEHDR * p_waveheader = (WAVEHDR *) malloc(sizeof(WAVEHDR));
> +    struct lkwavehdr * p_waveheader = 
> +        (struct lkwavehdr *) malloc(sizeof(struct lkwavehdr));
>      if(!p_waveheader)
>      {
>          msg_Err(p_aout, "Couldn't alloc WAVEHDR");
>          return;

Memory leak.

>      }
> +
> +    p_waveheader->p_next = NULL;
> +
>      if( block && p_aout->sys->chans_to_reorder )
>      {
>          aout_ChannelReorder( block->p_buffer, block->i_buffer,
> @@ -429,6 +442,8 @@ static void Play( audio_output_t *p_aout, block_t
> *block )
>          msleep( block->i_length );
>      }
>  
> +    WaveOutClean( p_aout->sys );
> +
>      vlc_mutex_lock( &p_aout->sys->lock );
>      p_aout->sys->i_frames++;
>      p_aout->sys->i_played_length += block->i_length;
> @@ -466,6 +481,10 @@ static void Stop( audio_output_t *p_aout )
>         }
>      }
>  
> +    /* wait for the frames to be queued in cleaning list */
> +    WaveOutFlush( p_aout, true );
> +    WaveOutClean( p_aout->sys );
> +
>      /* now we can Close the device */
>      if( waveOutClose( p_sys->h_waveout ) != MMSYSERR_NOERROR )
>      {
> @@ -639,15 +658,15 @@ static int OpenWaveOutPCM( audio_output_t *p_aout,
> uint32_t i_device_id,
>   * PlayWaveOut: play a buffer through the WaveOut device
>  
*****************************************************************************/
>  static int PlayWaveOut( audio_output_t *p_aout, HWAVEOUT h_waveout,
> -                        WAVEHDR *p_waveheader, block_t *p_buffer, bool
> b_spdif)
> +                        struct lkwavehdr *p_waveheader, block_t
> *p_buffer, bool b_spdif)
>  {
>      MMRESULT result;
>  
>      /* Prepare the buffer */
>      if( p_buffer != NULL )
>      {
> -        p_waveheader->lpData = (LPSTR)p_buffer->p_buffer;
> -        p_waveheader->dwBufferLength = p_buffer->i_buffer;
> +        p_waveheader->hdr.lpData = (LPSTR)p_buffer->p_buffer;
> +        p_waveheader->hdr.dwBufferLength = p_buffer->i_buffer;
>          /*
>            copy the buffer to the silence buffer :) so in case we don't
>            get the next buffer fast enough (I will repeat this one a
time
> @@ -672,14 +691,14 @@ static int PlayWaveOut( audio_output_t *p_aout,
> HWAVEOUT h_waveout,
>                             0x00, p_aout->sys->i_buffer_size );
>             }
>          }
> -        p_waveheader->lpData = (LPSTR)p_aout->sys->p_silence_buffer;
> -        p_waveheader->dwBufferLength = p_aout->sys->i_buffer_size;
> +        p_waveheader->hdr.lpData =
(LPSTR)p_aout->sys->p_silence_buffer;
> +        p_waveheader->hdr.dwBufferLength = p_aout->sys->i_buffer_size;
>      }
>  
> -    p_waveheader->dwUser = p_buffer ? (DWORD_PTR)p_buffer :
(DWORD_PTR)1;
> -    p_waveheader->dwFlags = 0;
> +    p_waveheader->hdr.dwUser = p_buffer ? (DWORD_PTR)p_buffer :
> (DWORD_PTR)1;
> +    p_waveheader->hdr.dwFlags = 0;
>  
> -    result = waveOutPrepareHeader( h_waveout, p_waveheader,
> sizeof(WAVEHDR) );
> +    result = waveOutPrepareHeader( h_waveout, &p_waveheader->hdr,
> sizeof(WAVEHDR) );
>      if( result != MMSYSERR_NOERROR )
>      {
>          msg_Err( p_aout, "waveOutPrepareHeader failed" );
> @@ -687,7 +706,7 @@ static int PlayWaveOut( audio_output_t *p_aout,
> HWAVEOUT h_waveout,
>      }
>  
>      /* Send the buffer to the waveOut queue */
> -    result = waveOutWrite( h_waveout, p_waveheader, sizeof(WAVEHDR) );
> +    result = waveOutWrite( h_waveout, &p_waveheader->hdr,
sizeof(WAVEHDR)
> );
>      if( result != MMSYSERR_NOERROR )
>      {
>          msg_Err( p_aout, "waveOutWrite failed" );
> @@ -704,21 +723,38 @@ static void CALLBACK WaveOutCallback( HWAVEOUT
> h_waveout, UINT uMsg,
>                                        DWORD_PTR _p_aout,
>                                        DWORD_PTR dwParam1, DWORD_PTR
>                                        dwParam2 )
>  {
> -    (void)dwParam2;
> +    (void) h_waveout;
> +    (void) dwParam2;
>      audio_output_t *p_aout = (audio_output_t *)_p_aout;
> -    WAVEHDR * p_waveheader =  (WAVEHDR *) dwParam1;
> +    struct lkwavehdr * p_waveheader =  (struct lkwavehdr *) dwParam1;
>  
>      if( uMsg != WOM_DONE ) return;
>  
> -    WaveOutClearBuffer( h_waveout, p_waveheader );
> -
> -    free(p_waveheader);
>      vlc_mutex_lock( &p_aout->sys->lock );
> +    p_waveheader->p_next = p_aout->sys->p_free_list;
> +    p_aout->sys->p_free_list = p_waveheader;
>      p_aout->sys->i_frames--;
>      vlc_cond_broadcast( &p_aout->sys->cond );
>      vlc_mutex_unlock( &p_aout->sys->lock );
>  }
>  
> +static void WaveOutClean( aout_sys_t * p_sys )
> +{
> +    struct lkwavehdr * p_whdr = NULL;
> +
> +    vlc_mutex_lock(&p_sys->lock);
> +    while( p_sys->p_free_list )
> +    {
> +        p_whdr = p_sys->p_free_list;
> +        p_sys->p_free_list = p_whdr->p_next;
> +        vlc_mutex_unlock(&p_sys->lock);
> +        WaveOutClearBuffer( p_sys->h_waveout, &p_whdr->hdr );
> +        free(p_whdr);
> +        vlc_mutex_lock(&p_sys->lock);
> +    }
> +    vlc_mutex_unlock(&p_sys->lock);
> +}

You don't need to reacquire the lock, just copy p_free_list to a local
variable and then set the former to NULL.

> +
>  static void WaveOutClearBuffer( HWAVEOUT h_waveout, WAVEHDR
*p_waveheader
>  )
>  {   
>      block_t *p_buffer = (block_t *)(p_waveheader->dwUser);
> @@ -873,6 +909,7 @@ static void WaveOutFlush( audio_output_t *p_aout,
bool
> wait)
>  static void WaveOutPause( audio_output_t * p_aout, bool pause, mtime_t
>  date)
>  {
>      MMRESULT res;
> +    (void) date;
>      if(pause)
>      {
>          res = waveOutPause( p_aout->sys->h_waveout );

-- 
Rémi Denis-Courmont
Sent from my collocated server