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 <phensman@webrtc.org>
Commit-Queue: Magnus Jedvert <magjed@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22629}diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn
index 5b48aae..d226d7d 100644
--- a/sdk/android/BUILD.gn
+++ b/sdk/android/BUILD.gn
@@ -198,6 +198,7 @@
":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 0e6b7be..c619878 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 @@
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<uint32_t> AAudioPlayer::SpeakerVolume() const {
+ return rtc::nullopt;
+}
+
+rtc::Optional<uint32_t> AAudioPlayer::MaxSpeakerVolume() const {
+ return rtc::nullopt;
+}
+
+rtc::Optional<uint32_t> 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 ce99797..c23081a 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 <aaudio/AAudio.h>
#include <memory>
+#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 @@
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<uint32_t> SpeakerVolume() const;
+ rtc::Optional<uint32_t> MaxSpeakerVolume() const;
+ rtc::Optional<uint32_t> 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 b6e8e9a..938a57b 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 @@
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 @@
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<uint32_t> 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<uint32_t> 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<uint32_t> min_volume = output_->MinSpeakerVolume();
+ if (!min_volume)
return -1;
- }
+ *output_min_volume = *min_volume;
return 0;
}
@@ -513,12 +514,8 @@
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 de3d7f4..48aab44 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 @@
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 @@
: -1;
}
-int AudioTrackJni::MaxSpeakerVolume(uint32_t* max_volume) const {
+rtc::Optional<uint32_t> AudioTrackJni::MaxSpeakerVolume() const {
RTC_DCHECK(thread_checker_.CalledOnValidThread());
- *max_volume = Java_WebRtcAudioTrack_getStreamMaxVolume(env_, j_audio_track_);
+ return Java_WebRtcAudioTrack_getStreamMaxVolume(env_, j_audio_track_);
+}
+
+rtc::Optional<uint32_t> AudioTrackJni::MinSpeakerVolume() const {
+ RTC_DCHECK(thread_checker_.CalledOnValidThread());
return 0;
}
-int AudioTrackJni::MinSpeakerVolume(uint32_t* min_volume) const {
+rtc::Optional<uint32_t> 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 db08c01..c73bd70 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 <jni.h>
#include <memory>
+#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 @@
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<uint32_t> SpeakerVolume() const;
+ rtc::Optional<uint32_t> MaxSpeakerVolume() const;
+ rtc::Optional<uint32_t> 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 5e9dfe0..d2b4299 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 @@
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<uint32_t> OpenSLESPlayer::SpeakerVolume() const {
+ return rtc::nullopt;
}
-int OpenSLESPlayer::MaxSpeakerVolume(uint32_t* maxVolume) const {
- return -1;
+rtc::Optional<uint32_t> OpenSLESPlayer::MaxSpeakerVolume() const {
+ return rtc::nullopt;
}
-int OpenSLESPlayer::MinSpeakerVolume(uint32_t* minVolume) const {
- return -1;
+rtc::Optional<uint32_t> 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 357d5e5..70629ea 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 <SLES/OpenSLES_AndroidConfiguration.h>
#include <memory>
+#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 @@
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<uint32_t> SpeakerVolume() const;
+ rtc::Optional<uint32_t> MaxSpeakerVolume() const;
+ rtc::Optional<uint32_t> MinSpeakerVolume() const;
void AttachAudioBuffer(AudioDeviceBuffer* audioBuffer);
