[vlc-devel] [PATCH] Erase regularily the already played directsound buffer outside of the play callback

[Denis Charmet]

Hi,

Thanks for the review.

Le jeudi 24 avril 2014 à 10:26:48, Rémi Denis-Courmont a écrit :
> >+        vlc_mutex_lock( &sys->lock );
> >+        sys->b_playing = true;
> >+        vlc_cond_broadcast(&sys->cond);
> 
> NIT: By convention, signal is used when only one thread waits on the
> variable.
> 
Indeed it's a bad reflex.
> >+        vlc_mutex_unlock( &sys->lock );
> >+
> >+    }
> >     return dsresult;
> > }
> >
> >@@ -298,6 +295,14 @@ static HRESULT Pause( aout_stream_sys_t *sys,
> >bool pause )
> >         hr = IDirectSoundBuffer_Stop( sys->p_dsbuffer );
> >     else
> >         hr = IDirectSoundBuffer_Play( sys->p_dsbuffer, 0, 0,
> >DSBPLAY_LOOPING );
> >+    if( hr == DS_OK )
> >+    {
> >+        vlc_mutex_lock( &sys->lock );
> >+        sys->b_playing = !pause;
> >+        if( sys->b_playing )
> >+            vlc_cond_broadcast( &sys->cond );
> >+        vlc_mutex_unlock( &sys->lock );
> >+    }
> >     return hr;
> > }
> >
> >@@ -513,6 +518,12 @@ static HRESULT CreateDSBufferPCM( vlc_object_t
> >*obj, aout_stream_sys_t *sys,
> >  */
> > static HRESULT Stop( aout_stream_sys_t *p_sys )
> > {
> >+    vlc_mutex_lock( &p_sys->lock );
> >+    p_sys->b_playing =  true;
> >+    vlc_cond_broadcast( &p_sys->cond );
> 
> Leaving cancellation enabled in vlc_cond_wait() would be simpler IMHO.
> 
I have to admit that thread cancellation is quite a mystery to me
especially with Windows threads...
> >+    vlc_cancel( p_sys->eraser_thread );
> >+    vlc_mutex_unlock( &p_sys->lock );
> >+    vlc_join( p_sys->eraser_thread, NULL );
> >     if( p_sys->p_notify != NULL )
> >     {
> >         IDirectSoundNotify_Release(p_sys->p_notify );
> >@@ -729,7 +740,27 @@ static HRESULT Start( vlc_object_t *obj,
> >aout_stream_sys_t *sys,
> >         }
> >     }
> >
> >+    int ret = vlc_clone(&sys->eraser_thread, PlayedDataEraser,
> >(void*) obj,
> >+                        VLC_THREAD_PRIORITY_LOW);
> >+    if( unlikely( ret ) )
> >+    {
> >+        if( ret != ENOMEM )
> >+            msg_Err( obj, "Couldn't start eraser thread" );
> >+        if( sys->p_notify != NULL )
> >+        {
> >+            IDirectSoundNotify_Release( sys->p_notify );
> >+            sys->p_notify = NULL;
> >+        }
> >+        IDirectSoundBuffer_Release( sys->p_dsbuffer );
> >+        sys->p_dsbuffer = NULL;
> >+        IDirectSound_Release( sys->p_dsobject );
> >+        sys->p_dsobject = NULL;
> >+        return ret;
> >+    }
> >+    sys->b_playing = false;
> >     sys->i_write = 0;
> >+    vlc_mutex_unlock( &sys->lock );
> >+
> >     return DS_OK;
> >
> > error:
> >@@ -1028,6 +1059,9 @@ static int Open(vlc_object_t *obj)
> >     aout_DeviceReport(aout, dev);
> >     free(dev);
> >
> >+    vlc_mutex_init(&sys->s.lock);
> >+    vlc_cond_init(&sys->s.cond);
> >+
> >     return VLC_SUCCESS;
> > }
> >
> >@@ -1035,8 +1069,83 @@ static void Close(vlc_object_t *obj)
> > {
> >     audio_output_t *aout = (audio_output_t *)obj;
> >     aout_sys_t *sys = aout->sys;
> >+    vlc_cond_destroy( &sys->s.cond );
> >+    vlc_mutex_destroy( &sys->s.lock );
> >
> >     var_Destroy(aout, "directx-audio-device");
> >     FreeLibrary(sys->hdsound_dll); /* free DSOUND.DLL */
> >     free(sys);
> > }
> >+
> >+static void * PlayedDataEraser( void * data )
> >+{
> >+    const audio_output_t *aout = (audio_output_t *) data;
> >+    aout_stream_sys_t *p_sys = &aout->sys->s;
> >+    void *p_write_position, *p_wrap_around;
> >+    unsigned long l_bytes1, l_bytes2;
> >+    DWORD i_read;
> >+    int64_t toerase, tosleep;
> >+    HRESULT dsresult;
> >+
> >+    for(;;)
> >+    {
> >+        int canc = vlc_savecancel();
> >+        vlc_mutex_lock( &p_sys->lock );
> >+
> >+        while( !p_sys->b_playing )
> >+           vlc_cond_wait( &p_sys->cond, &p_sys->lock );
> >+
> >+        toerase = 0;
> >+        tosleep = 0;
> >+
> >+        dsresult = IDirectSoundBuffer_GetCurrentPosition(
> >p_sys->p_dsbuffer,
> >+
> >&i_read, NULL );
> >+        if( dsresult == DS_OK )
> >+        {
> >+            int64_t max = (int64_t) i_read - (int64_t)
> >p_sys->i_write;
> >+            tosleep = -max;
> >+            if( max <= 0 )
> >+                max += DS_BUF_SIZE;
> >+            else
> >+                tosleep += DS_BUF_SIZE;
> >+            toerase = max;
> >+            tosleep = ( tosleep / p_sys->i_bytes_per_sample ) *
> >CLOCK_FREQ / p_sys->i_rate;
> >+        }
> >+
> >+        tosleep = __MAX( tosleep, 20000 );
> >+        dsresult = IDirectSoundBuffer_Lock( p_sys->p_dsbuffer,
> >+                                            p_sys->i_write,
> >+                                            toerase,
> >+                                            &p_write_position,
> >+                                            &l_bytes1,
> >+                                            &p_wrap_around,
> >+                                            &l_bytes2,
> >+                                            0 );
> >+        if( dsresult == DSERR_BUFFERLOST )
> >+        {
> >+            IDirectSoundBuffer_Restore( p_sys->p_dsbuffer );
> >+            dsresult = IDirectSoundBuffer_Lock( p_sys->p_dsbuffer,
> >+                                                p_sys->i_write,
> >+                                                toerase,
> >+                                                &p_write_position,
> >+                                                &l_bytes1,
> >+                                                &p_wrap_around,
> >+                                                &l_bytes2,
> >+                                                0 );
> >+        }
> >+        if( dsresult != DS_OK )
> >+            goto wait;
> >+
> >+        memset( p_write_position, 0, l_bytes1 );
> >+        memset( p_wrap_around, 0, l_bytes2 );
> >+
> >+        IDirectSoundBuffer_Unlock( p_sys->p_dsbuffer,
> >p_write_position, l_bytes1,
> >+                                   p_wrap_around, l_bytes2 );
> >+wait:
> >+        vlc_mutex_unlock(&p_sys->lock);
> >+        vlc_restorecancel(canc);
> >+        msleep(tosleep);
> >+    }
> >+    vlc_mutex_unlock(&p_sys->lock);
> 
> Dead code?
> 
indeed
> >+    return NULL;
> >+}

Regards,

-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre