Set default sampling rate to 44100 and query the native output sampling rate.

Set default sampling rate to 44100 and query the native output sampling rate.

BUG=231094
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=200461

[leozwang1, 2013-04-15 15:11:26]

Tommi: please review in general.
Yaron: please also review android java code.
thanks.

[leozwang1, 2013-04-15 15:13:11]

+Wei, please review code in content and in general.

[DaleCurtis, 2013-04-15 18:13:06]

https://chromiumcodereview.appspot.com/14049003/diff/1/media/audio/audio_mana...
File media/audio/audio_manager_base.cc (right):

https://chromiumcodereview.appspot.com/14049003/diff/1/media/audio/audio_mana...
media/audio/audio_manager_base.cc:390: int
AudioManagerBase::GetNativeOutputSampleRatee() {
It makes me sad that this needs to be done here :(

How hard would it be to move all of the android stuff into AudioManagerAndroid?

One way would be for ReleaseOutputStream() and ReleaseInputStream() to return a
bool of true or false if there are any streams left.

Then AudioManagerAndroid just needs to override these methods and check the
return values to handle the android specific bits.  Doing this allows you to
keep all the android stuff in Android.

[DaleCurtis, 2013-04-15 18:13:34]

Also, is there a native buffer size we should be querying too? I thought some of
the Android guys mentioned that in the meeting last week.

[leozwang1, 2013-04-15 18:20:26]

On 2013/04/15 18:13:34, DaleCurtis wrote:
> Also, is there a native buffer size we should be querying too? I thought some
of
> the Android guys mentioned that in the meeting last week.

It will be in a separated CL.

[leozwang1, 2013-04-15 18:32:18]

It looks nicer and I can do it.
Shall I do it in this CL or a separated CL? Personally I feel it might be better
to clean it up in a separated CL, and make this CL only addresses sampling rate
issue. Please let me know.
https://code.google.com/p/chromium/issues/detail?id=231468

[DaleCurtis, 2013-04-15 18:34:47]

Refactoring it in another CL and then rebasing this one on top of that sounds
good to me.

[tommi (sloooow) - chröme, 2013-04-16 10:53:30]

I moved myself to cc since Wei is reviewing as well.  I've got a few things to
take care of so will use the extra minutes wisely ;-)

[leozwang1, 2013-04-16 18:15:20]

Dale and Wei, PTAL.

[DaleCurtis, 2013-04-16 20:08:15]

lgtm % nit. Looks much nicer with your refactoring, thanks!

https://chromiumcodereview.appspot.com/14049003/diff/15001/media/base/android...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://chromiumcodereview.appspot.com/14049003/diff/15001/media/base/android...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:21:
private static final int DEFAULT_SAMPLING_RATE = 44100;
Maybe a comment here on how this value was chosen?

[leozwang1, 2013-04-16 22:21:52]

https://chromiumcodereview.appspot.com/14049003/diff/15001/media/base/android...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://chromiumcodereview.appspot.com/14049003/diff/15001/media/base/android...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:21:
private static final int DEFAULT_SAMPLING_RATE = 44100;
On 2013/04/16 20:08:16, DaleCurtis wrote:
> Maybe a comment here on how this value was chosen?

Done.

https://chromiumcodereview.appspot.com/14049003/diff/23001/media/base/android...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://chromiumcodereview.appspot.com/14049003/diff/23001/media/base/android...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:23:
private static final int DEFAULT_SAMPLING_RATE = 44100;
I don't have solid data to support it, but 44.1K is used on a lot of android
devices.

[leozwang1, 2013-04-17 17:14:08]

+Raymond, seems he's working on the same issue.

[Raymond Toy (Google), 2013-04-17 17:29:29]

Thanks for doing this!

lgtm

https://chromiumcodereview.appspot.com/14049003/diff/23001/media/base/android...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://chromiumcodereview.appspot.com/14049003/diff/23001/media/base/android...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:89:
return Integer.parseInt(sampleRateString);
Is sampleRateString guaranteed not to be null?  The docs for getProperty says it
can return null if there is no value for that key.

[leozwang1, 2013-04-17 18:28:16]

thanks Raymond.

https://chromiumcodereview.appspot.com/14049003/diff/23001/media/base/android...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://chromiumcodereview.appspot.com/14049003/diff/23001/media/base/android...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:89:
return Integer.parseInt(sampleRateString);
On 2013/04/17 17:29:29, rtoy wrote:
> Is sampleRateString guaranteed not to be null?  The docs for getProperty says
it
> can return null if there is no value for that key.

Done.

[Raymond Toy (Google), 2013-05-15 23:19:46]

One minor comment

https://chromiumcodereview.appspot.com/14049003/diff/36001/media/audio/androi...
File media/audio/android/audio_manager_android.cc (right):

https://chromiumcodereview.appspot.com/14049003/diff/36001/media/audio/androi...
media/audio/android/audio_manager_android.cc:65: GetNativeOutputSampleRate(),
16, kDefaultBufferSize);
Is it right to set the input sample rate to the output sample rate?

[leozwang1, 2013-05-16 01:11:55]

https://chromiumcodereview.appspot.com/14049003/diff/36001/media/audio/androi...
File media/audio/android/audio_manager_android.cc (right):

https://chromiumcodereview.appspot.com/14049003/diff/36001/media/audio/androi...
media/audio/android/audio_manager_android.cc:65: GetNativeOutputSampleRate(),
16, kDefaultBufferSize);
There is no api to query input sampling rate now. It's the best guess that SoC
will have same sampling rate for both input and output, probably it's true on
most of google lead devices.

[Yaron, 2013-05-16 01:15:33]

-me, +Min

[qinmin, 2013-05-16 01:31:23]

lgtm

https://chromiumcodereview.appspot.com/14049003/diff/36001/media/base/android...
File media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java
(right):

https://chromiumcodereview.appspot.com/14049003/diff/36001/media/base/android...
media/base/android/java/src/org/chromium/media/AudioManagerAndroid.java:87:
String sampleRateString = mAudioManager.getProperty(
nit: No need for 2 if statement:

String sampleRateString = null;
if (...)
  sampleRateString = ....;
return sampleRateString == null ? DEFAULT_SAMPLING_RATE :
Integer.parseInt(sampleRateString);

[commit-bot: I haz the power, 2013-05-16 02:15:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/leozwang@chromium.org/14049003/36001

[commit-bot: I haz the power, 2013-05-16 05:34:19]

Change committed as 200461