Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppRTC: URL param stereo=true does not result in stereo due to APM still enabled. #220

Closed
KaptenJansson opened this issue Nov 17, 2014 · 10 comments

Comments

@KaptenJansson
Copy link
Contributor

The URL param Stereo=true does not enable stereo all the way (just sets the stereo=1 in the SDP) in chrome unless echoCancellation=false is sent in as an optional gUM constraint.

This is due to the APM does not handle stereo hence it's down-mixed to mono.
The URL params &stereo=true, audio=echoCancellation=false will result in stereo.

If the intention of stereo=true is to enable proper stereo (not just setting the SDP attribute) it should also pass the audio=echoCancellation=false gum constraint.

Justin, WDYT?

@marwahvikas
Copy link
Contributor

It seems like we should turn off echoCancellation when stereo is false. Maybe we should have some notification in apprtc that since you passed stereo to true, the echocancellation has been turned off.

@KaptenJansson
Copy link
Contributor Author

Ping

@KaptenJansson KaptenJansson assigned samdutton and unassigned juberti Jan 16, 2015
@KaptenJansson
Copy link
Contributor Author

sam, maybe you could take a look? If not could you please reassign to someone who can.

@samdutton
Copy link
Contributor

Just to be clear – echoCancellation must be set to false whenever stereo is set to true?

This should be fairly straightforward to do in main.js via setCodecParam() (right?) but what is the value?

Maybe we should have some notification in apprtc that since you passed stereo to true, the echocancellation has been turned off.

Agree – via the infobox.

@juberti

@KaptenJansson
Copy link
Contributor Author

On Fri Jan 16 2015 at 3:07:31 PM Sam Dutton notifications@github.com
wrote:

Just to be clear – echoCancellation must be set to false whenever stereo
is set to true?
Yes.

This should be fairly straightforward to do in main.js via setCodecParam()
(right?) but what is the value?
It is an optional audio gum constraint, not a codec param (I've not looked at the code so not sure if setCodecParam() does other things as well).

Maybe we should have some notification in apprtc that since you passed
stereo to true, the echocancellation has been turned off.

Agree – via the infobox.

@juberti https://github.com/juberti


Reply to this email directly or view it on GitHub
#220 (comment).

@juberti
Copy link
Contributor

juberti commented Jan 20, 2015

Is AEC the only thing that doesn't tolerate stereo? Or does all APM processing have to be disabled? @KaptenJansson can you check with Tina?

@KaptenJansson
Copy link
Contributor Author

I believe the APM converts to mono before processing the audio hence it
affects all processing. But I will double check with Tina.

On Tue Jan 20 2015 at 8:56:17 PM Justin Uberti notifications@github.com
wrote:

Is AEC the only thing that doesn't tolerate stereo? Or does all APM
processing have to be disabled? @KaptenJansson
https://github.com/KaptenJansson can you check with Tina?


Reply to this email directly or view it on GitHub
#220 (comment).

@jiayliu
Copy link
Contributor

jiayliu commented Feb 4, 2015

Should this be closed?

@juberti
Copy link
Contributor

juberti commented Feb 4, 2015

Let's transfer this to the new tracker

@albrgbrg
Copy link

The URL param Stereo=true does not enable stereo all the way (just sets the stereo=1 in the SDP) in chrome unless echoCancellation=false is sent in as an optional gUM constraint.

This is due to the APM does not handle stereo hence it's down-mixed to mono.
The URL params &stereo=true, audio=echoCancellation=false will result in stereo.

If the intention of stereo=true is to enable proper stereo (not just setting the SDP attribute) it should also pass the audio=echoCancellation=false gum constraint.

Justin, WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants