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 override initial consumers MID values on WebRTC transport. #584

Closed
wants to merge 2 commits into from

Conversation

mstyura
Copy link
Contributor

@mstyura mstyura commented Jun 10, 2021

I'm working on project which uses media soup as SFU on server side. Media Soup clients libraries are not used and own signaling is being implemented.

Each client uses single peer connection in unified plan mode to send and receive media, in contrast to media soup demo app, which has distinct PC to send and receive media. The clients are supposed to have single PC with 2 m= sections in sendonly mode to send media, and N m= section in recvonly/inactive to receive media.

On server side for each client single WebRTCTransport created. 2 producers created for audio&video m= sections from SDP offer from client. Those producers are usually created with mid 0 and 1 (because that's how chrome and firefox generate SDP). The consumers for other producers on same router are also created starting with mid 0, 1, 2, ... on that transport.
Later when RTP parameters of consumers and producers are transformed into single SDP with count(producers) m= sections in sendonly mode and count(consumers) m= sections in recvonly mode there is an issue: because consumers and producers has overlapping mid (in my scenario when I don't deal with sendrecv m= sections).

It would be nice to have a way to control initial mid value of created consumers just to make IDs non overlapping.
So this PR proposes one of the solution to my problem.

@jmillan
Copy link
Member

jmillan commented Jun 11, 2021

Hi @mstyura,

This approach won't work if producers and consumers are not created just in that order. Ie, the following scenario won't work:

  • Create producer
  • Create consumer
  • Create producer
  • Create consumer

So this solution is only valid for your use case and hence can't be adopted.

Maybe an innocuous approach would be just prepending Consumer's rtpParameters.mid with a prefix like consumer- or c-.

I'll let @ibc comment on this.

Copy link
Member

@ibc ibc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Jose. This PR takes too many assumptions based on a single and specific use case. If we want MID to be provided by the app it should be an addition to the transport.consume() API and test units are needed.

Closing since we cannot adopt this solution.

@ibc ibc closed this Jun 11, 2021
@mstyura
Copy link
Contributor Author

mstyura commented Jun 11, 2021

@jmillan @ibc thank you very much for feedback. I'll try to implement solution suggested by @ibc by changing transport.consume() API and will send another PR.

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

Successfully merging this pull request may close these issues.

3 participants