From b1d454e9a59d482da63eba58ac6e08654524c996 Mon Sep 17 00:00:00 2001 From: "derat@chromium.org" Date: Wed, 23 Feb 2011 02:39:46 +0000 Subject: chromeos: Check ALSA return values in mixer code. We're seeing a NaN value sometimes get saved (incorrectly) to the prefs file in the volume setting, which results in the prefs file being unparseable the next time Chrome starts, which results in us going through OOBE again. The prefs code ought to write NaNs correctly so that they don't corrupt the file, but in the meantime, this change checks the return values from a bunch of ALSA functions (my best theory as to how the NaN is creeping in) and for good measure, also maps NaN volume values to the minimum volume. BUG=chromium-os:12229 TEST=built and ran it (i haven't been able to repro the problem) Review URL: http://codereview.chromium.org/6562001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@75698 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/chromeos/audio_mixer_alsa.cc | 103 +++++++++++++++++++++------- chrome/browser/chromeos/audio_mixer_alsa.h | 1 + 2 files changed, 79 insertions(+), 25 deletions(-) diff --git a/chrome/browser/chromeos/audio_mixer_alsa.cc b/chrome/browser/chromeos/audio_mixer_alsa.cc index 70c47a5..0b8d788 100644 --- a/chrome/browser/chromeos/audio_mixer_alsa.cc +++ b/chrome/browser/chromeos/audio_mixer_alsa.cc @@ -4,6 +4,8 @@ #include "chrome/browser/chromeos/audio_mixer_alsa.h" +#include + #include #include "base/logging.h" @@ -125,8 +127,13 @@ void AudioMixerAlsa::SetVolumeDb(double vol_db) { base::AutoLock lock(mixer_state_lock_); if (mixer_state_ != READY) return; - if (vol_db < kSilenceDb) + + if (vol_db < kSilenceDb || isnan(vol_db)) { + if (isnan(vol_db)) + LOG(WARNING) << "Got request to set volume to NaN"; vol_db = kSilenceDb; + } + DoSetVolumeDb_Locked(vol_db); prefs_->SetDouble(prefs::kAudioVolume, vol_db); } @@ -270,7 +277,14 @@ bool AudioMixerAlsa::InitializeAlsaMixer() { if (elem_master_) { alsa_long_t long_lo = static_cast(kDefaultMinVolume * 100); alsa_long_t long_hi = static_cast(kDefaultMaxVolume * 100); - snd_mixer_selem_get_playback_dB_range(elem_master_, &long_lo, &long_hi); + err = snd_mixer_selem_get_playback_dB_range( + elem_master_, &long_lo, &long_hi); + if (err != 0) { + LOG(WARNING) << "snd_mixer_selem_get_playback_dB_range() failed " + << "for master: " << snd_strerror(err); + snd_mixer_close(handle); + return false; + } min_volume_ = static_cast(long_lo) / 100.0; max_volume_ = static_cast(long_hi) / 100.0; } else { @@ -283,7 +297,13 @@ bool AudioMixerAlsa::InitializeAlsaMixer() { if (elem_pcm_) { alsa_long_t long_lo = static_cast(kDefaultMinVolume * 100); alsa_long_t long_hi = static_cast(kDefaultMaxVolume * 100); - snd_mixer_selem_get_playback_dB_range(elem_pcm_, &long_lo, &long_hi); + err = snd_mixer_selem_get_playback_dB_range(elem_pcm_, &long_lo, &long_hi); + if (err != 0) { + LOG(WARNING) << "snd_mixer_selem_get_playback_dB_range() failed for PCM: " + << snd_strerror(err); + snd_mixer_close(handle); + return false; + } min_volume_ += static_cast(long_lo) / 100.0; max_volume_ += static_cast(long_hi) / 100.0; } @@ -312,7 +332,7 @@ void AudioMixerAlsa::DoSetVolumeMute(double pref_volume, int pref_mute) { // If volume or mute are invalid, set them now to the current actual values. if (!PrefVolumeValid(pref_volume)) pref_volume = DoGetVolumeDb_Locked(); - bool mute; + bool mute = false; if (pref_mute == kPrefMuteInvalid) mute = GetElementMuted_Locked(elem_master_); else @@ -348,10 +368,11 @@ void AudioMixerAlsa::RestoreVolumeMuteOnUIThread() { double AudioMixerAlsa::DoGetVolumeDb_Locked() const { double vol_total = 0.0; - GetElementVolume_Locked(elem_master_, &vol_total); + if (!GetElementVolume_Locked(elem_master_, &vol_total)) + return 0.0; double vol_pcm = 0.0; - if (elem_pcm_ && (GetElementVolume_Locked(elem_pcm_, &vol_pcm))) + if (elem_pcm_ && GetElementVolume_Locked(elem_pcm_, &vol_pcm)) vol_total += vol_pcm; return vol_total; @@ -396,11 +417,15 @@ snd_mixer_elem_t* AudioMixerAlsa::FindElementWithName_Locked( bool AudioMixerAlsa::GetElementVolume_Locked(snd_mixer_elem_t* elem, double* current_vol) const { alsa_long_t long_vol = 0; - snd_mixer_selem_get_playback_dB(elem, - static_cast(0), - &long_vol); - *current_vol = static_cast(long_vol) / 100.0; + int alsa_result = snd_mixer_selem_get_playback_dB( + elem, static_cast(0), &long_vol); + if (alsa_result != 0) { + LOG(WARNING) << "snd_mixer_selem_get_playback_dB() failed: " + << snd_strerror(alsa_result); + return false; + } + *current_vol = static_cast(long_vol) / 100.0; return true; } @@ -410,14 +435,27 @@ bool AudioMixerAlsa::SetElementVolume_Locked(snd_mixer_elem_t* elem, double rounding_bias) { alsa_long_t vol_lo = 0; alsa_long_t vol_hi = 0; - snd_mixer_selem_get_playback_volume_range(elem, &vol_lo, &vol_hi); + int alsa_result = + snd_mixer_selem_get_playback_volume_range(elem, &vol_lo, &vol_hi); + if (alsa_result != 0) { + LOG(WARNING) << "snd_mixer_selem_get_playback_volume_range() failed: " + << snd_strerror(alsa_result); + return false; + } alsa_long_t vol_range = vol_hi - vol_lo; if (vol_range <= 0) return false; alsa_long_t db_lo_int = 0; alsa_long_t db_hi_int = 0; - snd_mixer_selem_get_playback_dB_range(elem, &db_lo_int, &db_hi_int); + alsa_result = + snd_mixer_selem_get_playback_dB_range(elem, &db_lo_int, &db_hi_int); + if (alsa_result != 0) { + LOG(WARNING) << "snd_mixer_selem_get_playback_dB_range() failed: " + << snd_strerror(alsa_result); + return false; + } + double db_lo = static_cast(db_lo_int) / 100.0; double db_hi = static_cast(db_hi_int) / 100.0; double db_step = static_cast(db_hi - db_lo) / vol_range; @@ -429,7 +467,12 @@ bool AudioMixerAlsa::SetElementVolume_Locked(snd_mixer_elem_t* elem, alsa_long_t value = static_cast(rounding_bias + (new_vol - db_lo) / db_step) + vol_lo; - snd_mixer_selem_set_playback_volume_all(elem, value); + alsa_result = snd_mixer_selem_set_playback_volume_all(elem, value); + if (alsa_result != 0) { + LOG(WARNING) << "snd_mixer_selem_set_playback_volume_all() failed: " + << snd_strerror(alsa_result); + return false; + } VLOG(1) << "Set volume " << snd_mixer_selem_get_name(elem) << " to " << new_vol << " ==> " << (value - vol_lo) * db_step + db_lo @@ -437,10 +480,13 @@ bool AudioMixerAlsa::SetElementVolume_Locked(snd_mixer_elem_t* elem, if (actual_vol) { alsa_long_t volume = vol_lo; - snd_mixer_selem_get_playback_volume( - elem, - static_cast(0), - &volume); + alsa_result = snd_mixer_selem_get_playback_volume( + elem, static_cast(0), &volume); + if (alsa_result != 0) { + LOG(WARNING) << "snd_mixer_selem_get_playback_volume() failed: " + << snd_strerror(alsa_result); + return false; + } *actual_vol = db_lo + (volume - vol_lo) * db_step; VLOG(1) << "Actual volume " << snd_mixer_selem_get_name(elem) @@ -451,19 +497,26 @@ bool AudioMixerAlsa::SetElementVolume_Locked(snd_mixer_elem_t* elem, bool AudioMixerAlsa::GetElementMuted_Locked(snd_mixer_elem_t* elem) const { int enabled = 0; - snd_mixer_selem_get_playback_switch( - elem, - static_cast(0), - &enabled); + int alsa_result = snd_mixer_selem_get_playback_switch( + elem, static_cast(0), &enabled); + if (alsa_result != 0) { + LOG(WARNING) << "snd_mixer_selem_get_playback_switch() failed: " + << snd_strerror(alsa_result); + return false; + } return (enabled) ? false : true; } void AudioMixerAlsa::SetElementMuted_Locked(snd_mixer_elem_t* elem, bool mute) { int enabled = mute ? 0 : 1; - snd_mixer_selem_set_playback_switch_all(elem, enabled); - - VLOG(1) << "Set playback switch " << snd_mixer_selem_get_name(elem) - << " to " << enabled; + int alsa_result = snd_mixer_selem_set_playback_switch_all(elem, enabled); + if (alsa_result != 0) { + LOG(WARNING) << "snd_mixer_selem_set_playback_switch_all() failed: " + << snd_strerror(alsa_result); + } else { + VLOG(1) << "Set playback switch " << snd_mixer_selem_get_name(elem) + << " to " << enabled; + } } } // namespace chromeos diff --git a/chrome/browser/chromeos/audio_mixer_alsa.h b/chrome/browser/chromeos/audio_mixer_alsa.h index 1626d29..eeb5b26 100644 --- a/chrome/browser/chromeos/audio_mixer_alsa.h +++ b/chrome/browser/chromeos/audio_mixer_alsa.h @@ -77,6 +77,7 @@ class AudioMixerAlsa : public AudioMixer { double rounding_bias); // In ALSA, the mixer element's 'switch' is turned off to mute. + // GetElementMuted_Locked() returns false on failure. bool GetElementMuted_Locked(_snd_mixer_elem* elem) const; void SetElementMuted_Locked(_snd_mixer_elem* elem, bool mute); -- cgit v1.1