Stability improvement for audio recording on Android

Stability improvement for audio recording on Android

BUG=NONE
R=magjed@webrtc.org

Committed: https://crrev.com/8a88dd271ed1091803ac5e4766f7023175d20b39
Cr-Commit-Position: refs/heads/master@{#10056}

[henrika_webrtc, 2015-09-24 13:14:27]

henrika@webrtc.org changed reviewers:
+ magjed@webrtc.org

[henrika_webrtc, 2015-09-24 13:16:40]

Patchset #2 (id:20001) has been deleted

[henrika_webrtc, 2015-09-24 13:30:25]

Many thanks magjed ;-)

[magjed_webrtc, 2015-09-24 13:35:13]

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java
(right):

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java:177:
minBufferSize == AudioRecord.ERROR_BAD_VALUE) {
|| should be first on the next line.

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java:185:
int bufferSizeInBytes = Math.max(byteBuffer.capacity(), 2 * minBufferSize);
Maybe split out the constant and comment into something like:
// Use a larger buffer size than the minimum required when creating the
// AudioRecord instance to ensure smooth recording under load. It has been
// verified that it does not increase the actual recording latency.
static final int BUFFER_SIZE_MULTIPLE = 2;
?

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java:216:
int bufferSizeInFrames = audioRecord.getBufferSizeInFrames();
I would remove this temporary variable and just inline
audioRecord.getBufferSizeInFrames() into the Logging statement.

[charlieloveeric3, 2015-09-24 13:36:20]

charlieloveeric3@gmail.com changed reviewers:
+ charlieloveeric3@gmail.com

[charlieloveeric3, 2015-09-24 13:36:20]

1
2
# this file is autogenerated by setup.py
version = "3.5.1"

[charlieloveeric3, 2015-09-24 13:39:01]

Index:
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
diff --git
a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
index  AIzaSyA3LdntxCxkeRf4gae6Q1yEqMVQb0QDeGw..
AIzaSyA3LdntxCxkeRf4gae6Q1yEqMVQb0QDeGw
---
a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
+++
b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
@@ -148,6 +148,11 @@ public final class WebRtcAudioUtils {
     return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP;
   }
 
+  public static boolean runningOnMOrHigher() {
+    // API Level 23.
+    return Build.VERSION.SDK_INT >= Build.VERSION_CODES.M;
+  }
+
   // Helper method for building a string of thread information.
   public static String getThreadInfo() {
     return "@[name=" + Thread.currentThread().getName()

[charlieloveeric3, 2015-09-24 13:40:22]

On 2015/09/24 13:39:01, charlieloveeric3 wrote:
> Index:
>
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
> diff --git
>
a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
>
b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
> index  AIzaSyA3LdntxCxkeRf4gae6Q1yEqMVQb0QDeGw..
> AIzaSyA3LdntxCxkeRf4gae6Q1yEqMVQb0QDeGw
> ---
>
a/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
> +++
>
b/webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioUtils.java
> @@ -148,6 +148,11 @@ public final class WebRtcAudioUtils {
>      return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP;
>    }
>  
> +  public static boolean runningOnMOrHigher() {
> +    // API Level 23.
> +    return Build.VERSION.SDK_INT >= Build.VERSION_CODES.M;
> +  }
> +
>    // Helper method for building a string of thread information.
>    public static String getThreadInfo() {
>      return "@[name=" + Thread.currentThread().getName()

[henrika_webrtc, 2015-09-24 14:29:57]

Thanks. PTAL.

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java
(right):

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java:177:
minBufferSize == AudioRecord.ERROR_BAD_VALUE) {
On 2015/09/24 13:35:13, magjed_webrtc wrote:
> || should be first on the next line.

Done.

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java:185:
int bufferSizeInBytes = Math.max(byteBuffer.capacity(), 2 * minBufferSize);
Sorry, did improve comment in the next round ;-)
Will break out a constant.

https://codereview.webrtc.org/1363323002/diff/40001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java:216:
int bufferSizeInFrames = audioRecord.getBufferSizeInFrames();
On 2015/09/24 13:35:13, magjed_webrtc wrote:
> I would remove this temporary variable and just inline
> audioRecord.getBufferSizeInFrames() into the Logging statement.

Done.

[henrika_webrtc, 2015-09-24 14:30:11]

henrika@webrtc.org changed reviewers:
- charlieloveeric3@gmail.com

[henrika_webrtc, 2015-09-24 14:31:15]

https://codereview.webrtc.org/1363323002/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java
(left):

https://codereview.webrtc.org/1363323002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioEffects.java:299:
+ ", has control: " + aec.hasControl());
Removed. Does not add value. Sorry.

[magjed_webrtc, 2015-09-24 14:36:17]

lgtm

https://codereview.webrtc.org/1363323002/diff/80001/webrtc/modules/audio_devi...
File
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java
(right):

https://codereview.webrtc.org/1363323002/diff/80001/webrtc/modules/audio_devi...
webrtc/modules/audio_device/android/java/src/org/webrtc/voiceengine/WebRtcAudioRecord.java:43:
// buffer size). The extra space is allocated to guard against glitched under
s/glitched/glitches/g

[henrika_webrtc, 2015-09-24 14:45:21]

Committed patchset #5 (id:100001) manually as
8a88dd271ed1091803ac5e4766f7023175d20b39 (presubmit successful).

[commit-bot: I haz the power, 2015-09-24 14:45:26]

Patchset 5 (id:??) landed as
https://crrev.com/8a88dd271ed1091803ac5e4766f7023175d20b39
Cr-Commit-Position: refs/heads/master@{#10056}