[vlc-devel] [PATCH] Use waveOutSetVolume to set volume instead of pure software gain

[Rémi Denis-Courmont]

On Wed, 16 Jan 2013 00:47:16 +0100, Denis Charmet <typx at dinauz.org> wrote:
> @@ -111,8 +113,9 @@ struct aout_sys_t
>  
>      uint8_t *p_silence_buffer;              /* buffer we use to play
>      silence */
>  
> -    float soft_gain;
> -    bool soft_mute;
> +    bool b_soft;                            /* Use software gain */
> +    float f_volume;
> +    bool b_mute;

Suboptimal packing.

>      uint8_t chans_to_reorder;              /* do we need channel
>      reordering */
>      uint8_t chan_table[AOUT_CHAN_MAX];
> @@ -126,8 +129,6 @@ struct aout_sys_t
>      vlc_cond_t cond;
>  };
>  
> -#include "volume.h"
> -
> 
/*****************************************************************************
>   * Module descriptor
>  
*****************************************************************************/
> @@ -137,6 +138,9 @@ struct aout_sys_t
>                         "to apply.")
>  #define DEFAULT_AUDIO_DEVICE N_("Default Audio Device")
>  
> +#define VOLUME_TEXT N_("WaveOut audio volume")
> +#define VOLUME_LONGTEXT N_("Use this volume at start of the audio
output")

Use the exact same translated string as other outputs.

> +
>  vlc_module_begin ()
>      set_shortname( "WaveOut" )
>      set_description( N_("Win32 waveOut extension output") )
> @@ -147,7 +151,9 @@ vlc_module_begin ()
>      add_string( "waveout-audio-device", "wavemapper",
>                   DEVICE_TEXT, DEVICE_LONG, false )
>         change_string_cb( ReloadWaveoutDevices )
> -    add_sw_gain( )
> +
> +    add_float( "waveout-volume", 1.0f, VOLUME_TEXT, VOLUME_LONGTEXT,
true
> )
> +         change_float_range(0.0f, 2.0f)
>  
>      add_bool( "waveout-float32", true, FLOAT_TEXT, FLOAT_LONGTEXT, true
)
>  
> @@ -168,6 +174,9 @@ static int Start( audio_output_t *p_aout,
> audio_sample_format_t *restrict fmt )
>      p_aout->pause = WaveOutPause;
>      p_aout->flush = WaveOutFlush;
>  
> +    /* Default behaviour is to use software gain */
> +    p_aout->sys->b_soft = true;
> +
>      /*
>        check for configured audio device!
>      */
> @@ -273,7 +282,11 @@ static int Start( audio_output_t *p_aout,
> audio_sample_format_t *restrict fmt )
>          aout_FormatPrepare( fmt );
>          p_aout->sys->i_buffer_size = FRAME_SIZE *
fmt->i_bytes_per_frame;
>  
> -        aout_SoftVolumeStart( p_aout );
> +        if( waveoutcaps.dwSupport & WAVECAPS_VOLUME )
> +            p_aout->sys->b_soft = false;
> +
> +        WaveoutVolumeSet( p_aout,
> +                          p_aout->sys->b_mute? 0.0f :
> p_aout->sys->f_volume );

I think setting volume to zero on mute will confuse the state machine a
bit.

>  
>          p_aout->sys->b_spdif = false;
>      }
> @@ -850,7 +863,14 @@ static int Open(vlc_object_t *obj)
>      aout->sys = sys;
>      aout->start = Start;
>      aout->stop = Stop;
> -    aout_SoftVolumeInit(aout);
> +    aout->volume_set = WaveoutVolumeSet;
> +    aout->mute_set = WaveoutMuteSet;
> +
> +    sys->f_volume = var_InheritFloat(aout, "waveout-volume");
> +    sys->b_mute = var_InheritBool(aout, "mute");
> +
> +    aout_MuteReport(aout, sys->b_mute);
> +    aout_VolumeReport(aout, sys->f_volume );
>  
>      vlc_mutex_init( &sys->lock );
>      vlc_cond_init( &sys->cond );
> @@ -913,7 +933,7 @@ static void WaveOutPause( audio_output_t * p_aout,
> bool pause, mtime_t date)
>  {
>      MMRESULT res;
>      (void) date;
> -    if(pause)
> +    if( pause )
>      {
>          res = waveOutPause( p_aout->sys->h_waveout );
>          if( res != MMSYSERR_NOERROR )

Unrelated cosmetic change.

> @@ -932,3 +952,95 @@ static void WaveOutPause( audio_output_t * p_aout,
> bool pause, mtime_t date)
>          }
>      }
>  }
> +
> +static int WaveoutVolumeSet( audio_output_t *p_aout, float volume )
> +{
> +    aout_sys_t *sys = p_aout->sys;
> +
> +    if( sys->b_soft )
> +    {
> +        float gain = volume * volume * volume;
> +        if ( !sys->b_mute && aout_GainRequest( p_aout, gain ) )
> +            return -1;

This:

> +
> +        sys->f_volume = volume;
> +
> +        if( var_InheritBool( p_aout, "volume-save" ) )
> +            config_PutFloat( p_aout, "waveout-volume", volume );
> +
> +        aout_VolumeReport( p_aout, volume);

...could be factored regardless of b_soft.

> +        return 0;
> +    }
> +
> +    const HWAVEOUT hwo = sys->h_waveout;
> +
> +    uint32_t vol = lroundf( volume * 0xffff.fp0 );
> +
> +    if( !sys->b_mute )
> +    {
> +        if( vol > 0xffff )
> +        {
> +            vol = 0xffff;
> +            /* Apply software gain: volume^3 comes from volume.h */
> +            if( aout_GainRequest( p_aout, volume * volume * volume ) )
> +            {
> +                msg_Err(p_aout, "Couldn't set soft gain to %f", volume
);
> +                volume = 1.0f;
> +            }
> +        }
> +        else if( volume <= 1.0f && sys->f_volume > 1.0f )
> +        {
> +            /* Reset Gain to 1.0f */
> +            if( aout_GainRequest( p_aout, 1.0f ) )
> +                msg_Err(p_aout, "Couldn't set soft gain back to 1.0" );
> +        }

I do not believe that software gain is needed. In all previous VLC
versions, the then maximum VLC volume (400%) was mapped to the maximum
WaveOut volume. You would obtain the same scale with:

> +
> +        MMRESULT r = waveOutSetVolume(hwo, vol | (vol << 16));
> +        if(r != MMSYSERR_NOERROR)
> +        {
> +            msg_Err( p_aout, "waveOutSetVolume failed (%u)", r );
> +            return -1;
> +        }
> +    }
> +
> +    sys->f_volume = volume;
> +
> +    if( var_InheritBool( p_aout, "volume-save" ) )
> +        config_PutFloat( p_aout, "waveout-volume", volume );
> +
> +    aout_VolumeReport( p_aout, volume );
> +    return 0;
> +}
> +
> +static int WaveoutMuteSet( audio_output_t * p_aout, bool mute )
> +{
> +    aout_sys_t *sys = p_aout->sys;
> +
> +    if( sys->b_soft )
> +    {
> +        float gain = sys->f_volume * sys->f_volume * sys->f_volume; 
> +        if ( aout_GainRequest( p_aout, mute ? 0.f : gain ) )
> +            return -1;

This could be factored too...

> +        sys->b_mute = mute;
> +
> +        aout_MuteReport( p_aout, mute);
> +        return 0;
> +    }
> +
> +    const HWAVEOUT hwo = sys->h_waveout;
> +    uint32_t vol = mute ? 0 : lroundf( sys->f_volume * 0xffff.fp0 );
> +
> +    if( vol > 0xffff )
> +        vol = 0xffff;
> +
> +    MMRESULT r = waveOutSetVolume( hwo, vol | ( vol << 16 ) );
> +    if( r != MMSYSERR_NOERROR )
> +    {
> +        msg_Err( p_aout, "waveOutSetVolume failed (%u)", r );
> +        return -1;
> +    }
> +
> +    sys->b_mute = mute;
> +    aout_MuteReport( p_aout, mute );
> +    return 0;
> +}

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