[vlc-devel] [PATCH] Erase regularily the already played directsound buffer outside of the play callback
Rémi Denis-Courmont
remi at remlab.net
Thu Apr 17 13:52:37 CEST 2014
On Thu, 17 Apr 2014 00:43:52 +0200, Denis Charmet <typx at dinauz.org> wrote:
> Since Directsound is unable to detect an underrun while looping,
By definition, you cannot underrun if you loop...
You should not loop, though that has other issues.
> It may look overkill but this properly fix #11145 once and for all.
I don't see the point in using DirectSound in VLC. I don't know why it was
introduced when it was, and I don't see how, for a media player, it would
be any better than WaveOut.
> ---
> modules/audio_output/directsound.c | 134
> +++++++++++++++++++++++++++++--------
> 1 file changed, 107 insertions(+), 27 deletions(-)
>
> diff --git a/modules/audio_output/directsound.c
> b/modules/audio_output/directsound.c
> index 3cca59c..c89fdf6 100644
> --- a/modules/audio_output/directsound.c
> +++ b/modules/audio_output/directsound.c
> @@ -49,7 +49,7 @@ static HRESULT StreamStart( aout_stream_t *,
> audio_sample_format_t *,
> static HRESULT StreamStop( aout_stream_t * );
> static int ReloadDirectXDevices( vlc_object_t *, const char *,
> char ***, char *** );
> -
> +static void * PlayedDataEraser( void * );
> /* Speaker setup override options list */
> static const char *const speaker_list[] = { "Windows default", "Mono",
> "Stereo",
> "Quad", "5.1", "7.1" };
> @@ -112,6 +112,10 @@ typedef struct aout_stream_sys
> vlc_fourcc_t format;
>
> size_t i_write;
> +
> + bool b_running;
> + vlc_mutex_t lock;
> + vlc_thread_t eraser_thread;
> } aout_stream_sys_t;
>
> /**
> @@ -173,27 +177,15 @@ static HRESULT FillBuffer( vlc_object_t *obj,
> aout_stream_sys_t *p_sys,
> size_t towrite = (p_buffer != NULL) ? p_buffer->i_buffer :
> DS_BUF_SIZE;
> void *p_write_position, *p_wrap_around;
> unsigned long l_bytes1, l_bytes2;
> - DWORD i_read;
> - size_t i_size;
> - mtime_t i_buf = towrite;
> HRESULT dsresult;
>
> - /* Erase all the data already played */
> - 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;
> - if( max <= 0 )
> - max += DS_BUF_SIZE;
> - i_buf = max;
> - }
> + vlc_mutex_lock( &p_sys->lock );
>
> /* Before copying anything, we have to lock the buffer */
> dsresult = IDirectSoundBuffer_Lock(
> p_sys->p_dsbuffer, /* DS buffer */
> p_sys->i_write, /* Start offset */
> - i_buf, /* Number of bytes */
> + towrite, /* Number of bytes */
> &p_write_position, /* Address of lock start */
> &l_bytes1, /* Count of bytes locked before wrap
> around */
> &p_wrap_around, /* Buffer address (if wrap around) */
> @@ -205,7 +197,7 @@ static HRESULT FillBuffer( vlc_object_t *obj,
> aout_stream_sys_t *p_sys,
> dsresult = IDirectSoundBuffer_Lock(
> p_sys->p_dsbuffer,
> p_sys->i_write,
> - i_buf,
> + towrite,
> &p_write_position,
> &l_bytes1,
> &p_wrap_around,
> @@ -217,6 +209,7 @@ static HRESULT FillBuffer( vlc_object_t *obj,
> aout_stream_sys_t *p_sys,
> msg_Warn( obj, "cannot lock buffer" );
> if( p_buffer != NULL )
> block_Release( p_buffer );
> + vlc_mutex_unlock( &p_sys->lock );
> return dsresult;
> }
>
> @@ -232,18 +225,13 @@ static HRESULT FillBuffer( vlc_object_t *obj,
> aout_stream_sys_t *p_sys,
> p_sys->chans_to_reorder,
> p_sys->chan_table,
> p_sys->format );
>
> + memcpy( p_write_position, p_buffer->p_buffer, l_bytes1 );
> + if( p_wrap_around && l_bytes2 )
> + memcpy( p_wrap_around, p_buffer->p_buffer + l_bytes1,
> l_bytes2 );
>
> - i_size = ( p_buffer->i_buffer < l_bytes1 ) ? p_buffer->i_buffer
:
> l_bytes1;
> - memcpy( p_write_position, p_buffer->p_buffer, i_size );
> - memset( (uint8_t*) p_write_position + i_size, 0, l_bytes1 -
> i_size );
> + if( unlikely( ( l_bytes1 + l_bytes2 ) < p_buffer->i_buffer ) )
> + msg_Err( obj, "Buffer overrun");
>
> - if( l_bytes1 < p_buffer->i_buffer)
> - { /* Compute the remaining buffer space to be written */
> - i_buf = i_buf - i_size - (l_bytes1 - i_size);
> - i_size = ( p_buffer->i_buffer - l_bytes1 < l_bytes2 ) ?
> p_buffer->i_buffer - l_bytes1 : l_bytes2;
> - memcpy( p_wrap_around, p_buffer->p_buffer + l_bytes1,
i_size
> );
> - memset( (uint8_t*) p_wrap_around + i_size, 0, i_buf -
i_size
> );
> - }
> block_Release( p_buffer );
> }
>
> @@ -253,6 +241,7 @@ static HRESULT FillBuffer( vlc_object_t *obj,
> aout_stream_sys_t *p_sys,
>
> p_sys->i_write += towrite;
> p_sys->i_write %= DS_BUF_SIZE;
> + vlc_mutex_unlock( &p_sys->lock );
>
> return DS_OK;
> }
> @@ -261,7 +250,6 @@ static HRESULT Play( vlc_object_t *obj,
> aout_stream_sys_t *sys,
> block_t *p_buffer )
> {
> HRESULT dsresult;
> -
> dsresult = FillBuffer( obj, sys, p_buffer );
> if( dsresult != DS_OK )
> return dsresult;
> @@ -513,6 +501,15 @@ 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 );
> + if( p_sys->b_running )
> + {
> + p_sys->b_running = false;
> + vlc_mutex_unlock( &p_sys->lock );
> + vlc_join( p_sys->eraser_thread, NULL );
> + vlc_mutex_lock( &p_sys->lock );
Why?
> + }
else
> + vlc_mutex_unlock( &p_sys->lock );
> if( p_sys->p_notify != NULL )
> {
> IDirectSoundNotify_Release(p_sys->p_notify );
> @@ -729,7 +726,20 @@ static HRESULT Start( vlc_object_t *obj,
> aout_stream_sys_t *sys,
> }
> }
>
> + vlc_mutex_lock(&sys->lock);
Why that?
> + sys->b_running = true;
> + 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" );
> + sys->b_running = false;
> + goto error;
Dead lock.
> + }
> sys->i_write = 0;
> + vlc_mutex_unlock( &sys->lock );
> +
> return DS_OK;
>
> error:
> @@ -1028,6 +1038,8 @@ static int Open(vlc_object_t *obj)
> aout_DeviceReport(aout, dev);
> free(dev);
>
> + vlc_mutex_init(&sys->s.lock);
> +
> return VLC_SUCCESS;
> }
>
> @@ -1035,8 +1047,76 @@ static void Close(vlc_object_t *obj)
> {
> audio_output_t *aout = (audio_output_t *)obj;
> aout_sys_t *sys = aout->sys;
> + 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, status;
> + mtime_t toerase;
> + HRESULT dsresult;
> +
> + vlc_mutex_lock( &p_sys->lock );
> + while( p_sys->b_running )
You can either use VLC thread cancellation in which case the boolean is
useless, or the Win32 thread API in which case the invalid thread handle
value renders the boolean is useless.
Either way, the boolean seems useless.
> + {
> + dsresult = IDirectSoundBuffer_GetStatus( p_sys->p_dsbuffer,
> &status );
> + if( dsresult != DS_OK ||
> + ( status & DSBSTATUS_PLAYING ) != DSBSTATUS_PLAYING )
> + goto wait;
> +
> + toerase = 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;
> + if( max <= 0 )
> + max += DS_BUF_SIZE;
> + toerase = max;
> + }
> +
> + 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);
> + msleep(20000);
This is horrible. I would not allow such thing on a platform I cared for.
> + vlc_mutex_lock(&p_sys->lock);
> + }
> + vlc_mutex_unlock(&p_sys->lock);
> + return NULL;
> +}
--
Rémi Denis-Courmont
Sent from my collocated server
More information about the vlc-devel
mailing list