Improved error handling in AUAudioInputStream::Open() for Mac OSX

Improved error handling in AUAudioInputStream::Open() for Mac OSX

BUG=549021
Committed: https://crrev.com/e8f2e5dea0f6ecdbd7106754d81643d33b44471d
Cr-Commit-Position: refs/heads/master@{#373818}

[henrika (OOO until Aug 14), 2016-02-05 12:13:38]

Description was changed from

==========
Improved error handling in AUAudioInputStream::Open() for Mac OSX

BUG=
==========

to

==========
Improved error handling in AUAudioInputStream::Open() for Mac OSX

BUG=549021
==========

[henrika (OOO until Aug 14), 2016-02-05 12:13:39]

henrika@chromium.org changed reviewers:
+ tommi@chromium.org

[henrika (OOO until Aug 14), 2016-02-05 12:14:59]

As discussed. Goal is to add more unique error codes to our existing histogram
tacking error codes. That way we can increase our knowledge in what can cause
AUAudioInputStream::Open()  to fail.

[henrika (OOO until Aug 14), 2016-02-05 12:16:28]

https://codereview.chromium.org/1668373003/diff/1/media/audio/mac/audio_low_l...
File media/audio/mac/audio_low_latency_input_mac.cc (right):

https://codereview.chromium.org/1668373003/diff/1/media/audio/mac/audio_low_l...
media/audio/mac/audio_low_latency_input_mac.cc:146:
HandleError(kAudioUnitErr_InvalidParameter);
Adding one extra here since we can then see the relation (frequency) of this
error in relation to the others.

[tommi (sloooow) - chröme, 2016-02-05 12:29:30]

https://codereview.chromium.org/1668373003/diff/1/media/audio/mac/audio_low_l...
File media/audio/mac/audio_low_latency_input_mac.cc (right):

https://codereview.chromium.org/1668373003/diff/1/media/audio/mac/audio_low_l...
media/audio/mac/audio_low_latency_input_mac.cc:122:
HandleError(kAudioUnitErr_Initialized);
I'm not  sure this is a good idea.  HandleError() does more than just capture
the stats.  It actually fires an error which may close the stream.

For now though it makes sense to me to add a DCHECK at the beginning of this
function to catch programmer error:

DCHECK(!audio_unit_);

https://codereview.chromium.org/1668373003/diff/1/media/audio/mac/audio_low_l...
media/audio/mac/audio_low_latency_input_mac.cc:146:
HandleError(kAudioUnitErr_InvalidParameter);
On 2016/02/05 12:16:28, henrika wrote:
> Adding one extra here since we can then see the relation (frequency) of this
> error in relation to the others.

Acknowledged.

[henrika (OOO until Aug 14), 2016-02-05 12:50:51]

Done.

Had to change one error code since the previous one did not compile on all bots.
Not sure why but the exact value is not that essential.

FAILED: /b/build/goma/gomacc
../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF
obj/media/audio/mac/media.audio_low_latency_input_mac.o.d
-DV8_DEPRECATION_WARNINGS -DCLD_VERSION=2 -
../../media/audio/mac/audio_low_latency_input_mac.cc:169:17: error: use of
undeclared identifier 'kAudioComponentErr_InstanceInvalidated'
    HandleError(kAudioComponentErr_InstanceInvalidated);

https://codereview.chromium.org/1668373003/diff/1/media/audio/mac/audio_low_l...
File media/audio/mac/audio_low_latency_input_mac.cc (right):

https://codereview.chromium.org/1668373003/diff/1/media/audio/mac/audio_low_l...
media/audio/mac/audio_low_latency_input_mac.cc:122:
HandleError(kAudioUnitErr_Initialized);
On 2016/02/05 12:29:30, tommi-chromium wrote:
> I'm not  sure this is a good idea.  HandleError() does more than just capture
> the stats.  It actually fires an error which may close the stream.
> 
> For now though it makes sense to me to add a DCHECK at the beginning of this
> function to catch programmer error:
> 
> DCHECK(!audio_unit_);

Done.

[tommi (sloooow) - chröme, 2016-02-05 12:58:56]

lgtm

[henrika (OOO until Aug 14), 2016-02-05 13:58:05]

https://codereview.chromium.org/1668373003/diff/40001/media/audio/mac/audio_l...
File media/audio/mac/audio_low_latency_input_mac_unittest.cc (left):

https://codereview.chromium.org/1668373003/diff/40001/media/audio/mac/audio_l...
media/audio/mac/audio_low_latency_input_mac_unittest.cc:196:
EXPECT_FALSE(ais->Open());
We don't support this case on the output side or any other platform either.
Removing this test since we now fail (on DCHECK) if Open() is called twice in a
row without intermediate Close().

[tommi (sloooow) - chröme, 2016-02-05 14:06:51]

lgtm

https://codereview.chromium.org/1668373003/diff/40001/media/audio/mac/audio_l...
File media/audio/mac/audio_low_latency_input_mac_unittest.cc (left):

https://codereview.chromium.org/1668373003/diff/40001/media/audio/mac/audio_l...
media/audio/mac/audio_low_latency_input_mac_unittest.cc:196:
EXPECT_FALSE(ais->Open());
On 2016/02/05 13:58:05, henrika wrote:
> We don't support this case on the output side or any other platform either.
> Removing this test since we now fail (on DCHECK) if Open() is called twice in
a
> row without intermediate Close(). 

Agree.

[henrika (OOO until Aug 14), 2016-02-05 15:58:07]

The CQ bit was checked by henrika@chromium.org

[commit-bot: I haz the power, 2016-02-05 15:58:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1668373003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1668373003/40001

[commit-bot: I haz the power, 2016-02-05 16:09:09]

Description was changed from

==========
Improved error handling in AUAudioInputStream::Open() for Mac OSX

BUG=549021
==========

to

==========
Improved error handling in AUAudioInputStream::Open() for Mac OSX

BUG=549021
==========

[commit-bot: I haz the power, 2016-02-05 16:09:10]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-05 16:10:16]

Description was changed from

==========
Improved error handling in AUAudioInputStream::Open() for Mac OSX

BUG=549021
==========

to

==========
Improved error handling in AUAudioInputStream::Open() for Mac OSX

BUG=549021

Committed: https://crrev.com/e8f2e5dea0f6ecdbd7106754d81643d33b44471d
Cr-Commit-Position: refs/heads/master@{#373818}
==========

[commit-bot: I haz the power, 2016-02-05 16:10:17]

Patchset 3 (id:??) landed as
https://crrev.com/e8f2e5dea0f6ecdbd7106754d81643d33b44471d
Cr-Commit-Position: refs/heads/master@{#373818}