Remove device change deduplication based on device id.

Remove device change deduplication based on device id.

It looks like some software changes internal flows instead of
changing the device id.  In place of this deduplication logic
I've added a rate limit of 1 change per 250ms, which is enough
to fix the original issue of multiple events.

Also fixes an issue where we were firing device changes four
times (!) --[eRender, eCapture] * [eConsole, eCommunications].
The AudioManager only supports output device notifications so
we should only be firing the listener callback for eRender.

With the rate limiting logic and only firing notifications for
eRender, this takes us down from four device changes to just
1 for unplug / replug of a device.

BUG=506712
TEST=device changes work, single stream created per multiple events.
TBR=henrika

Committed: https://crrev.com/0a021e87fa8e211bdf5c9f9d2293f2d95fdd76dd
Cr-Commit-Position: refs/heads/master@{#339815}

[DaleCurtis, 2015-07-20 22:57:04]

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

[DaleCurtis, 2015-07-20 22:57:38]

WDYT? Simplifies the code a fair bit and greatly speeds up device changes. Will
have the user test this after landing.

[henrika_webrtc, 2015-07-21 09:42:16]

Sounds like a great idea to clean up this area. Thanks!

LGTM

[henrika_webrtc, 2015-07-21 09:42:23]

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

[henrika_webrtc, 2015-07-21 09:42:24]

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
File media/audio/win/audio_device_listener_win.cc (left):

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
media/audio/win/audio_device_listener_win.cc:72: default_render_device_id_ =
GetDeviceId(eRender, eConsole);
Nice to get rid of all these ;-)

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
File media/audio/win/audio_device_listener_win.cc (right):

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
media/audio/win/audio_device_listener_win.cc:151: now - last_device_change_time_
> base::TimeDelta::FromMilliseconds(250)) {
How did you come up with 250?

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
media/audio/win/audio_device_listener_win.cc:154: did_run_listener_cb = true;
Is did_run_listener_cb ever used?

[DaleCurtis, 2015-07-21 19:03:36]

Thanks for review!

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
File media/audio/win/audio_device_listener_win.cc (right):

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
media/audio/win/audio_device_listener_win.cc:151: now - last_device_change_time_
> base::TimeDelta::FromMilliseconds(250)) {
On 2015/07/21 09:42:23, henrika_webrtc wrote:
> How did you come up with 250?

Dice roll! Well actually... I physically couldn't plug/unplug devices any slower
than 250ms, so it seems reasonable.

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
media/audio/win/audio_device_listener_win.cc:154: did_run_listener_cb = true;
On 2015/07/21 09:42:23, henrika_webrtc wrote:
> Is did_run_listener_cb ever used?

In the DVLOG() below.

[DaleCurtis, 2015-07-21 19:03:43]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2015-07-21 19:04:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1248543002/1

[commit-bot: I haz the power, 2015-07-21 19:45:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-07-21 19:45:57]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[DaleCurtis, 2015-07-22 00:25:20]

The CQ bit was checked by dalecurtis@chromium.org

[DaleCurtis, 2015-07-22 00:25:21]

The patchset sent to the CQ was uploaded after l-g-t-m from henrika@webrtc.org
Link to the patchset: https://codereview.chromium.org/1248543002/#ps20001
(title: "Fix tests.")

[commit-bot: I haz the power, 2015-07-22 00:26:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1248543002/20001

[commit-bot: I haz the power, 2015-07-22 01:58:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-22 01:58:37]

Patchset 2 (id:??) landed as
https://crrev.com/0a021e87fa8e211bdf5c9f9d2293f2d95fdd76dd
Cr-Commit-Position: refs/heads/master@{#339815}

[Nico, 2015-07-22 02:25:30]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2015-07-22 02:25:31]

The win/clang bot complains:

..\..\media\audio\win\audio_device_listener_win_unittest.cc(24,19) : error:
unused variable 'kNoDevice' [-Werror,-Wunused-const-variable]
static const char kNoDevice[] = "";
                  ^

Can you fix?

[Nico, 2015-07-22 02:32:29]

…looks like you're out already. https://codereview.chromium.org/1250933002
fixes.

[henrika (OOO until Aug 14), 2015-07-22 07:58:20]

Minor comment.

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
File media/audio/win/audio_device_listener_win.cc (right):

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
media/audio/win/audio_device_listener_win.cc:151: now - last_device_change_time_
> base::TimeDelta::FromMilliseconds(250)) {
On 2015/07/21 19:03:36, DaleCurtis wrote:
> On 2015/07/21 09:42:23, henrika_webrtc wrote:
> > How did you come up with 250?
> 
> Dice roll! Well actually... I physically couldn't plug/unplug devices any
slower
> than 250ms, so it seems reasonable.

Acknowledged.

https://codereview.chromium.org/1248543002/diff/1/media/audio/win/audio_devic...
media/audio/win/audio_device_listener_win.cc:154: did_run_listener_cb = true;
On 2015/07/21 19:03:36, DaleCurtis wrote:
> On 2015/07/21 09:42:23, henrika_webrtc wrote:
> > Is did_run_listener_cb ever used?
> 
> In the DVLOG() below.

Acknowledged.

https://codereview.chromium.org/1248543002/diff/20001/media/audio/win/audio_d...
File media/audio/win/audio_device_listener_win_unittest.cc (right):

https://codereview.chromium.org/1248543002/diff/20001/media/audio/win/audio_d...
media/audio/win/audio_device_listener_win_unittest.cc:46: void
AdvanceLastDeviceChangeTime() {
Perhaps add a comment here to explain why you add 1 ms here. At least I will
forget the details ;-)