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

Add DSCP on RTP audio packets in SIP plugin #2150

Merged
merged 2 commits into from
May 11, 2020
Merged

Conversation

GerardM22
Copy link
Contributor

To complete #2055, I propose to do the same thing in the RTP audio packets sent by the SIP plugin.
Tested OK on my implementation of Janus server.

@januscla
Copy link

Thanks for your contribution, @GerardM22! Please make sure you sign our CLA, as it's a required step before we can merge this.

@lminiero
Copy link
Member

Why only audio?

@GerardM22
Copy link
Contributor Author

We only use audio in our Janus server but I can do the same thing for video.
I see that there are 4 types of stream in the SIP plugin:

  • RTP audio
  • RTP video
  • RTCP audio
  • RTCP video
    Do you think that it can be useful to do it for RTCP too or only for RTP ?

@lminiero
Copy link
Member

No idea if RTCP should have the same value: if you know what's typical in the industry you can try doing the same. But I'd definitely add an option for RTP video too. And there should probably a check on the setsockopt call: if that fails there should be at least a warning in the logs.

@lminiero
Copy link
Member

Any chance you can do the same for the NoSIP plugin too, now that you're at it since they're "related" as plugins and media-wise do many things the same way? Thanks!

PS: please don't forget about signing the CLA, as we can't merge otherwise.

@GerardM22
Copy link
Contributor Author

I added DSCP setting on video RTP socket in the the SIP plugin and the check of setsockopt as you suggested but I tested only the audio.
I think its not necessary to do it for RTCP sockets.
I never looked at the NoSIP plugin, so I prefer not to touch it for the moment.

@lminiero
Copy link
Member

I added DSCP setting on video RTP socket in the the SIP plugin and the check of setsockopt as you suggested but I tested only the audio.
I think its not necessary to do it for RTCP sockets.

Thanks!

I never looked at the NoSIP plugin, so I prefer not to touch it for the moment.

Fair enough, I'll add it myself then. This looks fine to me, so I'll merge, thanks again 👍

@lminiero lminiero merged commit ddbe10c into meetecho:master May 11, 2020
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.

None yet

3 participants