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

Simulcast Improvements #1345

Closed
6 of 7 tasks
Sean-Der opened this issue Jul 24, 2020 · 4 comments · Fixed by #1936
Closed
6 of 7 tasks

Simulcast Improvements #1345

Sean-Der opened this issue Jul 24, 2020 · 4 comments · Fixed by #1936
Assignees

Comments

@Sean-Der
Copy link
Member

Sean-Der commented Jul 24, 2020

Simulcast has landed! This a list of improvements I think would be great to see before /v3 is tagged.

  • Support SSRC Based Simulcast
    FireFox uses SSRCes instead of rid and mid. We should support this as well.

  • Refactor/Cleanup RTPReceiver
    Rename trackStreams -> readerStreams. A lot more stuff will fall into place when multiple onTrack paths are removed.

  • Update examples/simulcast to respond to remote Session Description
    Currently the example is tightly coupled with the frontend code. It would be nice if the backend dynamically constructed tracks depending on how many Simulcast layers the offerer is sending.

  • Implementing ReadRTCP for Simulcast Streams
    Update Read to allow users to pass in the SSRC as well? Maybe we don't break the API and assume readerStreams[0] if no SSRC is passed? 6540022

  • Remove multiple OnTrack paths
    Remove the goroutine from startReceiver. Everything should instead just react it incoming SSRCes. We could rename handleUndeclaredSSRC to be something like onNewIncomingSSRC.

  • E2E Tests
    Currently we just have unit testing, but no examples of E2E testing. Would be fine to send packets manually now. If easy might be worth adding proper Simulcast client support.

  • Fix Simulcast Probing deadlock
    Currently if a sender doesn't send enough packets handleUndeclaredSSRC can hang forever. This prevents probing taking place on other streams. We should fire a goroutine per stream we want to probe. 70fb90c

@Sean-Der
Copy link
Member Author

@jbrady42 do you have any bandwidth to help with these? (if not that is ok!)

Fantastic work on the Simulcast so far, I was super excited to merge it :) I tried to cleanup stuff as I went.

@Sean-Der
Copy link
Member Author

Sean-Der commented Oct 3, 2020

I am taking this, will work on this during the weekend!

Sean-Der added a commit that referenced this issue Dec 7, 2020
When a new SSRC is seen we start a Read loop for the packets. However if
we only see one packet this loop will just sit forever. If a user
doesn't send us enough packets to finish probing it will prevent any
subsequent streams from being probed.

Relates to #1345
Sean-Der added a commit that referenced this issue Dec 7, 2020
When a new SSRC is seen we start a Read loop for the packets. However if
we only see one packet this loop will just sit forever. If a user
doesn't send us enough packets to finish probing it will prevent any
subsequent streams from being probed.

Relates to #1345
Sean-Der added a commit that referenced this issue Dec 7, 2020
When a new SSRC is seen we start a Read loop for the packets. However if
we only see one packet this loop will just sit forever. If a user
doesn't send us enough packets to finish probing it will prevent any
subsequent streams from being probed.

Relates to #1345
@cameronelliott
Copy link

Issue/sub-issue

tweaking the simulcast example so it doesn't feed the media back to the browser causes an issue where OnTrack has blank Rids for the simulcast tracks

Correct output from simulcast

Connection State has changed connected
Track has started
Sending pli for stream with rid: "f", ssrc: 610171300
Sending remb for stream with rid: "f", ssrc: 610171300
Track has started
Track has started
Sending pli for stream with rid: "f", ssrc: 610171300
Sending remb for stream with rid: "f", ssrc: 610171300
Sending pli for stream with rid: "h", ssrc: 4056720671

simulcast that demos bug with blank/empty Rids:

Connection State has changed connected
Track has started  rid is
Track has started  rid is
Sending pli for stream with rid: "", ssrc: 133249352
Sending remb for stream with rid: "", ssrc: 133249352
Sending pli for stream with rid: "", ssrc: 3288342995
Sending remb for stream with rid: "", ssrc: 3288342995
Track has started  rid is
Sending pli for stream with rid: "", ssrc: 133249352
Sending remb for stream with rid: "", ssrc: 133249352

Repo with issue, last commit is changes to realize the bug:
(look at last commit to see my small changes vs. the vanilla simulcast)
https://github.com/cameronelliott/webrtc.empty-rids

Jsfiddle needed for issue: (also in the repo linked)
https://jsfiddle.net/camfoo/p2xn4kw5/2/

@jongwow
Copy link

jongwow commented Feb 15, 2021

@cameronelliott
Hi! I'm not familiar with open source and English isn't my first language, so please understand.. 😢

I think that happens when there is only one media on SDP.
In case of simulcast, on sdp only one media(video) is described but actually more than one(depending on how many encodings)
Check this peerconnection code.
It doesn't create ssrcs we need(it is optional according to this issue).

If you disabled above peerConnection code, it gets proper rid and creates ssrc.

Sean-Der added a commit that referenced this issue Aug 26, 2021
Sean-Der added a commit that referenced this issue Aug 26, 2021
@Sean-Der Sean-Der added this to the 3.1.0 milestone Aug 27, 2021
Sean-Der added a commit that referenced this issue Aug 27, 2021
Sean-Der added a commit that referenced this issue Aug 28, 2021
Sean-Der added a commit that referenced this issue Aug 29, 2021
Sean-Der added a commit that referenced this issue Aug 30, 2021
Will be used by Simulcast renegotation tests as well

Relates to #1345
daonb pushed a commit to daonb/webrtc that referenced this issue Nov 20, 2022
daonb pushed a commit to daonb/webrtc that referenced this issue Nov 20, 2022
daonb pushed a commit to daonb/webrtc that referenced this issue Nov 20, 2022
daonb pushed a commit to daonb/webrtc that referenced this issue Nov 20, 2022
Will be used by Simulcast renegotation tests as well

Relates to pion#1345
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants