If a cubeb_stream is stopped when a default device change event is received, the existing handler reconfigures *and* restarts the stream. The handler should pay attention to the current state of the stream rather than blindly restarting it.

Comment on attachment 8565795 [details] [diff] [review] Don't restart stopped cubeb streams when handling device change notifications This fixes a few issues I ran into during recent testing. Restarting the stream when it's stopped caused various problems all over the media code, including leaked render threads and a fatal assert on EOS. While working on this, I noticed that OnDefaultDeviceChanged seems to be called twice on my Windows 8.1 machine for each change. The flow, role, and device_id are identical on each call. Other than being suboptimal, I haven't found any problems (or a way to prevent it) yet.

Minor build fix; applied patch copied from another tree incorrectly.

Looks like the tests are hanging on shutdown, will investigate after dinner.

Missed updating an assert, but I can't see it firing in the try logs... https://treeherder.mozilla.org/#/jobs?repo=try&revision=60b0ad5dd053

Comment on attachment 8565908 [details] [diff] [review] bug1134078_v1.patch Review of attachment 8565908 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/libcubeb/src/cubeb_wasapi.cpp @@ +30,5 @@ > #ifndef STACK_SIZE_PARAM_IS_A_RESERVATION > #define STACK_SIZE_PARAM_IS_A_RESERVATION 0x00010000 // Threads only > #endif > > +#define LOGGING_ENABLED (don't forget to revert this) @@ +585,5 @@ > } > break; > + case WAIT_TIMEOUT: > + assert(GetThreadId(stm->thread) == GetCurrentThreadId() && > + stm->shutdown_event == wait_array[0]); Do we want to call the state callback if this happens? My patch for this set hr to a negative value so we fell in the CUBEB_STATE_STOPPED below.

(In reply to Paul Adenot (:padenot) from comment #11) > Do we want to call the state callback if this happens? My patch for this set > hr to a negative value so we fell in the CUBEB_STATE_STOPPED below. Yeah, that makes sense to me. I was assuming we'd end up merging both patches, net addition from me then being just the assert (hoping to gain a lead on bug 1108793).

FWIW the XP mochitests were failing because GetThreadId() isn't available before Vista, so there was a popup on the desktop with a dynamic linker error... took me a while to track that down. Also, per comment 8 and bug 1133386, the asserts didn't show up in the log because they also cause a popup on the desktop.