1266438 - Reset the AEC on device change

Status: RESOLVED FIXED

(Core :: WebRTC: Audio/Video, defect)

Comment 1

[Paul Adenot (:padenot)]

Attachment: Part 1, reset the processing

Comment 2

[Paul Adenot (:padenot)]

Attachment: Part 2, remove OSX speicifc workarounds

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8743927 [details] [diff] [review]
Part 1, reset the processing

Review of attachment 8743927 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/GraphDriver.cpp
@@ +22,5 @@
>  #define STREAM_LOG(type, msg) MOZ_LOG(gMediaStreamGraphLog, type, msg)
>  
>  // We don't use NSPR log here because we want this interleaved with adb logcat
>  // on Android/B2G
> +#define ENABLE_LIFECYCLE_LOG

will need to comment this again

::: dom/media/MediaStreamGraph.cpp
@@ +45,5 @@
>  
>  LazyLogModule gMediaStreamGraphLog("MediaStreamGraph");
>  #define STREAM_LOG(type, msg) MOZ_LOG(gMediaStreamGraphLog, type, msg)
>  
> +#define ENABLE_LIFECYCLE_LOG

ditto

::: dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ +502,5 @@
> +#define ResetProcessingIfNeeded(_processing)                      \
> +do {                                                              \
> +webrtc::_processing##Modes mode;                                  \
> +int rv = mVoEProcessing->Get##_processing##Status(enabled, mode); \
> +if (rv) {                                                         \

indent contents of do {} block

@@ +504,5 @@
> +webrtc::_processing##Modes mode;                                  \
> +int rv = mVoEProcessing->Get##_processing##Status(enabled, mode); \
> +if (rv) {                                                         \
> +  NS_WARNING("Could not get the status of the "                   \
> +   #_processing "on device change.");                             \

Add a space before 'on'

@@ +521,5 @@
> +  if (rv) {                                                       \
> +    NS_WARNING("Could not reset the status of the "               \
> +    #_processing " on device change.");                           \
> +    return;                                                       \
> +  }                                                               \

Issues with enabled and doing this if already false (per IRC)

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8743928 [details] [diff] [review]
Part 2, remove OSX speicifc workarounds

Review of attachment 8743928 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/GraphDriver.cpp
@@ +1005,5 @@
>  #endif
>  }
>  
>  void
> +  AudioCallbackDriver::DeviceChangedCallback() {

spurious spaces

Comment 5

[Paul Adenot (:padenot)]

Attachment: Part 1, reset the processing

Comment 6

[Paul Adenot (:padenot)]

Attachment: Part 2, remove OSX specific workarounds

Addressed nits.

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8743949 [details] [diff] [review]
Part 1, reset the processing

Review of attachment 8743949 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/webrtc/MediaEngineWebRTCAudio.cpp
@@ +509,5 @@
> +    return;                                                         \
> +  }                                                                 \
> +                                                                    \
> +  if (enabled) {                                                    \
> +    rv = mVoEProcessing->Set##_processing##Status(!enabled);        \

false

@@ +516,5 @@
> +      #_processing " on device change.");                           \
> +      return;                                                       \
> +    }                                                               \
> +                                                                    \
> +    rv = mVoEProcessing->Set##_processing##Status(enabled);         \

true

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5663efa6c13c
https://hg.mozilla.org/integration/mozilla-inbound/rev/099d4414bd63

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5663efa6c13c
https://hg.mozilla.org/mozilla-central/rev/099d4414bd63