From ff4cac9c48e71119ea747c5db15b394244a52c84 Mon Sep 17 00:00:00 2001 From: Magnus Jedvert Date: Tue, 27 Mar 2018 15:15:41 +0200 Subject: [PATCH] Android audio device template: Don't use output parameters Our style guide dictates that we should prefer using return values rather than output parameters when we can. Some of the methods like MaxSpeakerVolume() are not required to be able to provide a value. In these cases I changed the return type to an rtc::Optional. Also, this CL fixes a bug with StereoRecordingIsAvailable() that would not previously be passed along correctly in the template layer. Bug: webrtc:7452 Change-Id: I0a1f455093bfe092627118d65a996212a65eeb2b Reviewed-on: https://webrtc-review.googlesource.com/64401 Reviewed-by: Paulina Hensman Commit-Queue: Magnus Jedvert Cr-Commit-Position: refs/heads/master@{#22629} --- sdk/android/BUILD.gn | 1 + .../src/jni/audio_device/aaudio_player.cc | 21 ++++++++++-- .../src/jni/audio_device/aaudio_player.h | 11 ++++--- .../audio_device_template_android.h | 33 +++++++++---------- .../src/jni/audio_device/audio_track_jni.cc | 26 +++++++-------- .../src/jni/audio_device/audio_track_jni.h | 9 ++--- .../src/jni/audio_device/opensles_player.cc | 17 +++++----- .../src/jni/audio_device/opensles_player.h | 9 ++--- 8 files changed, 70 insertions(+), 57 deletions(-) diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index 5b48aae76e..d226d7d63a 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -198,6 +198,7 @@ rtc_source_set("audio_device_jni") { ":generated_audio_jni", ":native_api_jni", "../../api:array_view", + "../../api:optional", "../../modules/audio_device:audio_device", "../../modules/audio_device:audio_device_buffer", "../../modules/utility:utility", diff --git a/sdk/android/src/jni/audio_device/aaudio_player.cc b/sdk/android/src/jni/audio_device/aaudio_player.cc index 0e6b7be4db..c619878704 100644 --- a/sdk/android/src/jni/audio_device/aaudio_player.cc +++ b/sdk/android/src/jni/audio_device/aaudio_player.cc @@ -129,9 +129,24 @@ void AAudioPlayer::AttachAudioBuffer(AudioDeviceBuffer* audioBuffer) { audio_device_buffer_, audio_parameters.sample_rate(), capacity)); } -int AAudioPlayer::SpeakerVolumeIsAvailable(bool* available) { - *available = false; - return 0; +bool AAudioPlayer::SpeakerVolumeIsAvailable() { + return false; +} + +int AAudioPlayer::SetSpeakerVolume(uint32_t volume) { + return -1; +} + +rtc::Optional AAudioPlayer::SpeakerVolume() const { + return rtc::nullopt; +} + +rtc::Optional AAudioPlayer::MaxSpeakerVolume() const { + return rtc::nullopt; +} + +rtc::Optional AAudioPlayer::MinSpeakerVolume() const { + return rtc::nullopt; } void AAudioPlayer::OnErrorCallback(aaudio_result_t error) { diff --git a/sdk/android/src/jni/audio_device/aaudio_player.h b/sdk/android/src/jni/audio_device/aaudio_player.h index ce99797e85..c23081a703 100644 --- a/sdk/android/src/jni/audio_device/aaudio_player.h +++ b/sdk/android/src/jni/audio_device/aaudio_player.h @@ -14,6 +14,7 @@ #include #include +#include "api/optional.h" #include "modules/audio_device/audio_device_buffer.h" #include "modules/audio_device/include/audio_device_defines.h" #include "rtc_base/messagehandler.h" @@ -70,11 +71,11 @@ class AAudioPlayer final : public AAudioObserverInterface, void AttachAudioBuffer(AudioDeviceBuffer* audioBuffer); // Not implemented in AAudio. - int SpeakerVolumeIsAvailable(bool* available); // NOLINT - int SetSpeakerVolume(uint32_t volume) { return -1; } - int SpeakerVolume(uint32_t* volume) const { return -1; } // NOLINT - int MaxSpeakerVolume(uint32_t* maxVolume) const { return -1; } // NOLINT - int MinSpeakerVolume(uint32_t* minVolume) const { return -1; } // NOLINT + bool SpeakerVolumeIsAvailable(); + int SetSpeakerVolume(uint32_t volume); + rtc::Optional SpeakerVolume() const; + rtc::Optional MaxSpeakerVolume() const; + rtc::Optional MinSpeakerVolume() const; protected: // AAudioObserverInterface implementation. diff --git a/sdk/android/src/jni/audio_device/audio_device_template_android.h b/sdk/android/src/jni/audio_device/audio_device_template_android.h index b6e8e9ac23..938a57b227 100644 --- a/sdk/android/src/jni/audio_device/audio_device_template_android.h +++ b/sdk/android/src/jni/audio_device/audio_device_template_android.h @@ -349,9 +349,7 @@ class AudioDeviceTemplateAndroid : public AudioDeviceModule { int32_t SpeakerVolumeIsAvailable(bool* available) override { RTC_LOG(INFO) << __FUNCTION__; CHECKinitialized_(); - if (output_->SpeakerVolumeIsAvailable(available) == -1) { - return -1; - } + *available = output_->SpeakerVolumeIsAvailable(); RTC_LOG(INFO) << "output: " << *available; return 0; } @@ -362,31 +360,34 @@ class AudioDeviceTemplateAndroid : public AudioDeviceModule { return output_->SetSpeakerVolume(volume); } - int32_t SpeakerVolume(uint32_t* volume) const override { + int32_t SpeakerVolume(uint32_t* output_volume) const override { RTC_LOG(INFO) << __FUNCTION__; CHECKinitialized_(); - if (output_->SpeakerVolume(volume) == -1) { + rtc::Optional volume = output_->SpeakerVolume(); + if (!volume) return -1; - } + *output_volume = *volume; RTC_LOG(INFO) << "output: " << *volume; return 0; } - int32_t MaxSpeakerVolume(uint32_t* maxVolume) const override { + int32_t MaxSpeakerVolume(uint32_t* output_max_volume) const override { RTC_LOG(INFO) << __FUNCTION__; CHECKinitialized_(); - if (output_->MaxSpeakerVolume(maxVolume) == -1) { + rtc::Optional max_volume = output_->MaxSpeakerVolume(); + if (!max_volume) return -1; - } + *output_max_volume = *max_volume; return 0; } - int32_t MinSpeakerVolume(uint32_t* minVolume) const override { + int32_t MinSpeakerVolume(uint32_t* output_min_volume) const override { RTC_LOG(INFO) << __FUNCTION__; CHECKinitialized_(); - if (output_->MinSpeakerVolume(minVolume) == -1) { + rtc::Optional min_volume = output_->MinSpeakerVolume(); + if (!min_volume) return -1; - } + *output_min_volume = *min_volume; return 0; } @@ -513,12 +514,8 @@ class AudioDeviceTemplateAndroid : public AudioDeviceModule { int32_t StereoRecordingIsAvailable(bool* available) const override { RTC_LOG(INFO) << __FUNCTION__; CHECKinitialized_(); - bool isAvailable = false; - if (audio_manager_.IsStereoRecordSupported() == -1) { - return -1; - } - *available = isAvailable; - RTC_LOG(INFO) << "output: " << isAvailable; + *available = audio_manager_.IsStereoRecordSupported(); + RTC_LOG(INFO) << "output: " << *available; return 0; } diff --git a/sdk/android/src/jni/audio_device/audio_track_jni.cc b/sdk/android/src/jni/audio_device/audio_track_jni.cc index de3d7f4907..48aab4467b 100644 --- a/sdk/android/src/jni/audio_device/audio_track_jni.cc +++ b/sdk/android/src/jni/audio_device/audio_track_jni.cc @@ -115,9 +115,8 @@ int32_t AudioTrackJni::StopPlayout() { return 0; } -int AudioTrackJni::SpeakerVolumeIsAvailable(bool* available) { - *available = true; - return 0; +bool AudioTrackJni::SpeakerVolumeIsAvailable() { + return true; } int AudioTrackJni::SetSpeakerVolume(uint32_t volume) { @@ -128,23 +127,22 @@ int AudioTrackJni::SetSpeakerVolume(uint32_t volume) { : -1; } -int AudioTrackJni::MaxSpeakerVolume(uint32_t* max_volume) const { +rtc::Optional AudioTrackJni::MaxSpeakerVolume() const { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + return Java_WebRtcAudioTrack_getStreamMaxVolume(env_, j_audio_track_); +} + +rtc::Optional AudioTrackJni::MinSpeakerVolume() const { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - *max_volume = Java_WebRtcAudioTrack_getStreamMaxVolume(env_, j_audio_track_); return 0; } -int AudioTrackJni::MinSpeakerVolume(uint32_t* min_volume) const { +rtc::Optional AudioTrackJni::SpeakerVolume() const { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - *min_volume = 0; - return 0; -} - -int AudioTrackJni::SpeakerVolume(uint32_t* volume) const { - RTC_DCHECK(thread_checker_.CalledOnValidThread()); - *volume = Java_WebRtcAudioTrack_getStreamVolume(env_, j_audio_track_); + const uint32_t volume = + Java_WebRtcAudioTrack_getStreamVolume(env_, j_audio_track_); RTC_LOG(INFO) << "SpeakerVolume: " << volume; - return 0; + return volume; } // TODO(henrika): possibly add stereo support. diff --git a/sdk/android/src/jni/audio_device/audio_track_jni.h b/sdk/android/src/jni/audio_device/audio_track_jni.h index db08c011d5..c73bd70f36 100644 --- a/sdk/android/src/jni/audio_device/audio_track_jni.h +++ b/sdk/android/src/jni/audio_device/audio_track_jni.h @@ -14,6 +14,7 @@ #include #include +#include "api/optional.h" #include "modules/audio_device/audio_device_buffer.h" #include "modules/audio_device/include/audio_device_defines.h" #include "rtc_base/thread_checker.h" @@ -52,11 +53,11 @@ class AudioTrackJni { int32_t StopPlayout(); bool Playing() const { return playing_; } - int SpeakerVolumeIsAvailable(bool* available); + bool SpeakerVolumeIsAvailable(); int SetSpeakerVolume(uint32_t volume); - int SpeakerVolume(uint32_t* volume) const; - int MaxSpeakerVolume(uint32_t* max_volume) const; - int MinSpeakerVolume(uint32_t* min_volume) const; + rtc::Optional SpeakerVolume() const; + rtc::Optional MaxSpeakerVolume() const; + rtc::Optional MinSpeakerVolume() const; void AttachAudioBuffer(AudioDeviceBuffer* audioBuffer); diff --git a/sdk/android/src/jni/audio_device/opensles_player.cc b/sdk/android/src/jni/audio_device/opensles_player.cc index 5e9dfe090e..d2b42997ab 100644 --- a/sdk/android/src/jni/audio_device/opensles_player.cc +++ b/sdk/android/src/jni/audio_device/opensles_player.cc @@ -166,25 +166,24 @@ int OpenSLESPlayer::StopPlayout() { return 0; } -int OpenSLESPlayer::SpeakerVolumeIsAvailable(bool* available) { - *available = false; - return 0; +bool OpenSLESPlayer::SpeakerVolumeIsAvailable() { + return false; } int OpenSLESPlayer::SetSpeakerVolume(uint32_t volume) { return -1; } -int OpenSLESPlayer::SpeakerVolume(uint32_t* volume) const { - return -1; +rtc::Optional OpenSLESPlayer::SpeakerVolume() const { + return rtc::nullopt; } -int OpenSLESPlayer::MaxSpeakerVolume(uint32_t* maxVolume) const { - return -1; +rtc::Optional OpenSLESPlayer::MaxSpeakerVolume() const { + return rtc::nullopt; } -int OpenSLESPlayer::MinSpeakerVolume(uint32_t* minVolume) const { - return -1; +rtc::Optional OpenSLESPlayer::MinSpeakerVolume() const { + return rtc::nullopt; } void OpenSLESPlayer::AttachAudioBuffer(AudioDeviceBuffer* audioBuffer) { diff --git a/sdk/android/src/jni/audio_device/opensles_player.h b/sdk/android/src/jni/audio_device/opensles_player.h index 357d5e5984..70629eaa4c 100644 --- a/sdk/android/src/jni/audio_device/opensles_player.h +++ b/sdk/android/src/jni/audio_device/opensles_player.h @@ -16,6 +16,7 @@ #include #include +#include "api/optional.h" #include "modules/audio_device/audio_device_buffer.h" #include "modules/audio_device/fine_audio_buffer.h" #include "modules/audio_device/include/audio_device_defines.h" @@ -71,11 +72,11 @@ class OpenSLESPlayer { int StopPlayout(); bool Playing() const { return playing_; } - int SpeakerVolumeIsAvailable(bool* available); + bool SpeakerVolumeIsAvailable(); int SetSpeakerVolume(uint32_t volume); - int SpeakerVolume(uint32_t* volume) const; - int MaxSpeakerVolume(uint32_t* maxVolume) const; - int MinSpeakerVolume(uint32_t* minVolume) const; + rtc::Optional SpeakerVolume() const; + rtc::Optional MaxSpeakerVolume() const; + rtc::Optional MinSpeakerVolume() const; void AttachAudioBuffer(AudioDeviceBuffer* audioBuffer);