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

Allow layer selection for publisher #175

Merged
merged 1 commit into from
May 12, 2021

Conversation

dreamerns
Copy link
Contributor

@dreamerns dreamerns commented Mar 5, 2021

Related to: ionorg/ion-sfu#477

Description

  • Export Layer type
  • Allow tweaking encoding params per layer by using optional layer param. This can be used to disable certain layer (for example keep the high layer disabled and enable it only for the main speaker)
  • Added API to select which layer should be enabled.
  • Allow sending a message to 'ion-sfu' channel for the publisher to communicate layer selection
  • Support new message type and layer selection callback

Reference issue

Fixes #...

@dreamerns dreamerns changed the title Export Layer and allow updating params per layer Allow layer selection for publisher Mar 18, 2021
@billylindeman billylindeman requested a review from OrlandoCo March 18, 2021 22:53
@dreamerns dreamerns marked this pull request as ready for review March 26, 2021 21:19
@dreamerns dreamerns force-pushed the update-layer-encodings branch from 7499d33 to 98cfe8d Compare March 30, 2021 04:55
@tarrencev
Copy link
Contributor

tarrencev commented Apr 15, 2021

Just to make sure i understand, is this so the sfu can tell the client which layers to send?

@dreamerns
Copy link
Contributor Author

dreamerns commented Apr 16, 2021

Just to make sure i understand, is this so the sfu can tell the client which layers to send?

This change allows the publisher to select which layer(s) should be enabled. Right now if I select FHD, I will send 3 layers all the time. If I want to change that, I'll have to renegotiate. With this change, the publisher can enable/disable the layer on the fly. An additional message is needed because there is no (easy) way for ion-sfu to detect that layer has been disabled. Once we send message of new layer selection, ion-sfu will check all the down tracks and update accordingly. New channel message activeLayer is for informational purposes only, so that subscribers are aware.

@adwpc
Copy link
Contributor

adwpc commented Apr 16, 2021

This force subscriber to use one layer?

@tarrencev
Copy link
Contributor

Just to make sure i understand, is this so the sfu can tell the client which layers to send?

This change allows the publisher to select which layer(s) should be enabled. Right now if I select FHD, I will send 3 layers all the time. If I want to change that, I'll have to renegotiate. With this change, the publisher can enable/disable the layer on the fly. An additional message is needed because there is no (easy) way for ion-sfu to detect that layer has been disabled. Once we send message of new layer selection, ion-sfu will check all the down tracks and update accordingly. New channel message activeLayer is for informational purposes only, so that subscribers are aware.

I think the SFU needs to do active track detection. Even without this change it is necessary since the browser can automatically toggle layers without user input. I think @OrlandoCo has plans to implement support for it at some point. My preference would be to wait for that since it avoids a lot of clientside/signaling complexity

@billylindeman billylindeman self-requested a review April 29, 2021 23:07
* Export Layer type
* Allow tweaking encoding params per layer by using optional layer param. This can be used to disable certain layer (for example keep high layer disabled and enable it only for main speaker)

* Adding support for active layer channel message

Support legacy channel message
@dreamerns dreamerns force-pushed the update-layer-encodings branch from 9cd6c67 to 4e30f57 Compare May 5, 2021 20:45
@billylindeman billylindeman merged commit 2cd134d into ionorg:master May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants