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.

Sign up for GitHub

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

Jump to bottom

Spatial audio support in AudioBridge via stereo mixing #2446

Merged
merged 10 commits into from
May 27, 2021
Merged

Spatial audio support in AudioBridge via stereo mixing #2446

merged 10 commits into from
May 27, 2021

Conversation

lminiero
Copy link
Member

As the title suggests, this is a first attempt at using stereo mixing for the purpose of implementing some basic spatial audio support: the idea, basically, is to allow you to place different participants in different parts of the stereo mix. Please notice that the implementation is quite naive, so you shouldn't expect anything as sophisticated as the STEAK project, for instance; that said, it should still be a good starting point, and can always be improved in the future.

By default AudioBridge rooms continue working as before. If you create rooms with a spatial_audio set to true, though, then the mixer is instructed to work with stereo streams instead. You place participants using the spatial_position attribute, which can be set both when joining, and dynamically via configure requests: this spatial_position ranges from 0 (hard left) to 100 (hard right), and defaults to 50 (center), which means you have a good range of positions to play with. As I was saying the implementation is quite naive, and uses L/R gains to simulate this positioning. When a user is in the center, we apply a 50% gain on both sides (half volume on L and R), which changes the more left/right the user is (hard left = 100% gain on L, 0% on R). Not sure this is the right math to perform, and if we should always keep 100% on the side until we cross the center and get gurther from it: this is one of the areas where we can probably improve, and feedback would be welcome.

I've marked this PR as a draft because the work is not complete. Besides the considerations made above, at the moment when using stereo streams we're also completely ignoring the volume gain, for instance, which will of course need to be taken into account in the future. Looking forward to some feedback on whether you think this is useful, and how it can be improved.

@lminiero
Copy link
Member Author

Nobody cares?

@augustblack
Copy link
Contributor

Nice work! At first glance, stereo is working. I can pan left and right on each stream.

setting 'spatial_position' on the stream seems to have the correct effect (even when panning the input stereo stream)

@augustblack
Copy link
Contributor

In testing on a mac, I can send stereo. However, when I open the audiobridge demo, I get stereo in Firefox and Chrome, but mono in Safari.

@lminiero
Copy link
Member Author

Thanks for testing! On Safari, it looks like a browser-specific issue, especially if Chrome and Firefox do their job. If you find out what needs to change in the SDP to make it happy (if stereo WebRTC is supported by Safari at all), I'll fix it.

@augustblack
Copy link
Contributor

What should I be looking for in the SDP?
I notice that for firefox/chrome, I can send and receive in stereo without having to mess with SDP offer.
Should the same behavior be expected for Safari?

@danjenkins
Copy link
Contributor

I'm surprised you haven't had to mess with the sdp for chrome! You need to enable stereo in the sdp in the offer and answer for your publisher and subscriber, pretty sure there's an example of how to do that in the streaming demo

@lminiero
Copy link
Member Author

I'm surprised you haven't had to mess with the sdp for chrome! You need to enable stereo in the sdp in the offer and answer for your publisher and subscriber, pretty sure there's an example of how to do that in the streaming demo

The updated AudioBridge demo tries to do that automatically, now, using customizeSdp (assuming you enabled stereo via the related query string argument).

@augustblack
Copy link
Contributor

I was using the old audiobrdige.js from master.
With the customizeSdp that sets stereo, all seems to be working fine in safari.

@fippo
Copy link
Contributor

fippo commented Jan 4, 2021

see the lengthy discussion in https://bugs.chromium.org/p/webrtc/issues/detail?id=8133 wrt sdp munging.

@nerdbaggy
Copy link

This is great. I haven’t been able to confirm this, but it seems like when the incoming stream is stereo, the browser disables the AEC on the users device? It seems kinda hardware and browser dependent, not really sure.

@lminiero
Copy link
Member Author

lminiero commented Jan 7, 2021

@nerdbaggy see the link @fippo posted.

@lmangani
Copy link

Is there a suggested or best practice "max rate" for firing spatial_position configure events, or events in general?
Asking for a friend who's mapping this to a mouse position event ;)

@lminiero
Copy link
Member Author

You mean, a rate at which clients should send a "configure" to reposition themselves? I don't know as we never stressed that part, but definitely not more than 50 per second, since that's the amount of packets we process per second for each user, and so it would be silly to send 100 packets as half of those positions would never be enforced in the stereo stream.

@lminiero lminiero marked this pull request as ready for review May 4, 2021 14:44
@lminiero
Copy link
Member Author

lminiero commented May 4, 2021

I feel this is now ready, even if not perfect. I added some controls to the demo as well to make testing easier, so please play with this and let us know.

@lminiero lminiero removed the WIP label May 11, 2021
@lminiero lminiero changed the title Spatial audio support in AudioBridge via stereo mixing (WIP) Spatial audio support in AudioBridge via stereo mixing May 11, 2021
@lminiero
Copy link
Member Author

Merging.

@lminiero lminiero merged commit 5757a37 into master May 27, 2021
@lminiero lminiero deleted the audiobridge-stereo branch May 27, 2021 15:08
BogdanovKirill pushed a commit to 3dEYE/janus-gateway that referenced this pull request Jun 10, 2021
Squashed commit of the following:
69df84b 
commit 9c9d335
Author: Lionel Nicolas <lionelnicolas@users.noreply.github.com>
Date:   Thu Jun 10 03:04:43 2021 -0400

    Fix streaming plugin mutex unlock when disabling mountpoint (meetecho#2690)

commit 2d83e96
Author: Yurii Cherniavskyi <yurii.cherniavskyi@gmail.com>
Date:   Mon Jun 7 16:02:41 2021 +0300

    Fix SIP plugin unhold request docs typo (meetecho#2688)

commit 2cd0118
Author: August Black <augustblack@gmail.com>
Date:   Mon Jun 7 01:10:49 2021 -0700

    minor adjustment to the audiobridge docs (meetecho#2687)

commit de2117e
Author: nicolasduteil <nduteil@freedev.org>
Date:   Tue Jun 1 11:26:29 2021 +0200

    fix: [janus_sip] Fix "call_id" property in "missed_call" events (meetecho#2679)

commit 9eeeb38
Author: Alessandro Toppi <atoppi@meetecho.com>
Date:   Mon May 31 15:57:41 2021 +0200

    Fix status vector parsing for incoming twcc feedbacks (resolves meetecho#2677).

commit 8a25f6e
Merge: d3b39b9 394fb48
Author: Alessandro Toppi <atoppi@meetecho.com>
Date:   Fri May 28 13:29:54 2021 +0200

    Merge pull request meetecho#2675 from kmeyerhofer/actions/fix

    GH Actions, fix variable name

commit d3b39b9
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Fri May 28 11:09:30 2021 +0200

    Fixed race condition in VideoRoom

commit 394fb48
Author: Kurt Meyerhofer <k@kcmr.io>
Date:   Thu May 27 14:52:08 2021 -0600

    Fixes variable name.

commit b45cd37
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Thu May 27 18:31:55 2021 +0200

    Clarify that libnice 0.1.18 is recommended

commit 5757a37
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Thu May 27 17:08:17 2021 +0200

    Spatial audio support in AudioBridge via stereo mixing (meetecho#2446)

commit 161fe7a
Author: Luca Barbato <luca.barbato@gmail.com>
Date:   Thu May 27 15:29:01 2021 +0200

    Cleanup avformat-based preprocessors (meetecho#2665)

commit 7b010cd
Author: lucylu-star <78361868+lucylu-star@users.noreply.github.com>
Date:   Tue May 25 17:09:40 2021 +0800

    Fixed broken simulcast support in VideoCall plugin (meetecho#2671)

commit 4ae44a4
Author: nicolasduteil <nduteil@freedev.org>
Date:   Mon May 24 17:57:34 2021 +0200

    feat: support for custom call-id in subscribe request + add 'call_id' property to subscribe & notify related events (meetecho#2664)

commit 4294f20
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Mon May 24 11:02:48 2021 +0200

    Fixed missing macro when using pthread mutexes (fixes meetecho#2666)

commit f22ab0d
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Wed May 19 12:03:32 2021 +0200

    Fixed warning

commit b3f3f17
Author: Alessandro Toppi <atoppi@meetecho.com>
Date:   Tue May 18 12:10:47 2021 +0200

    Remove duplicated flag for fuzzing coverage.

commit 4a7560c
Author: nu774 <honeycomb77@gmail.com>
Date:   Fri May 14 00:26:36 2021 +0900

    janus-pp-rec: support HEVC AP(aggregation packet) (meetecho#2662)

commit 5db4be2
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Wed May 12 17:43:43 2021 +0200

    Fixed out of bounds array access

commit 69f56f4
Author: nicolasduteil <nduteil@freedev.org>
Date:   Tue May 11 14:36:22 2021 +0200

    feat: support for SUBSCRIBE expiry (Expires header) in sip plugin (meetecho#2661)

commit b047ccf
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Mon May 10 09:33:27 2021 +0200

    Fixed types

commit f8e8c5e
Author: Chris Wiggins <chris@wiggins.nz>
Date:   Mon May 10 19:26:45 2021 +1200

    RabbitMQ Transport Reconnect Logic (meetecho#2651)

commit 280e8e4
Author: Lorenzo Miniero <lminiero@gmail.com>
Date:   Fri May 7 12:54:30 2021 +0200

    Add per-participant recording options in AudioBridge to join API as well
@vincentfretin vincentfretin mentioned this pull request Dec 18, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
please-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lminiero @augustblack @danjenkins @fippo @nerdbaggy @lmangani