-
Notifications
You must be signed in to change notification settings - Fork 151
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
Dynacast #62
Dynacast #62
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good! Question, does this work across platforms with flutter-webrtc translating the RTPSender params?
lib/src/publication/local.dart
Outdated
// If there is exact match, use it | ||
if (e.quality.toRid() == encoding.rid) return true; | ||
// Use first layer found if not simulcast | ||
if (encoding.rid == null && encodings.length == 1) return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when RID isn't defined, we should specifically match it to LOW quality layer. The protocol doesn't guarantee layers to come in in any specific order. (even tho low comes in first).
return false; | ||
}); | ||
if (layer != null && encoding.active != layer.enabled) { | ||
encoding.active = layer.enabled; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since Flutter includes web, it'll likely suffer from the same issue as web SDK with FireFox. I think it'd be good to put a comment in here, but it's fine if we don't handle it right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is flutter-webrtc using Firefox's WebRTC library for web?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they simply translate into equivalent calls from dart to JS. So any browser specific implementation quirks will still remain
I hope it does a good job translating the params but if it doesn't we need to make a PR... |
LK-352
For non-simulcast, what is the desired behavior ?