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 video support #35

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add video support #35

wants to merge 1 commit into from

Conversation

Hollymingyi
Copy link

hi author,
this is my code added for video . but when it work , call from sipjs softphone into livekit ,the video of softphone is very fuzzy, i dont know how to handle it . please give me some advice , thank you.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


zhanghe2 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link

codecov bot commented Dec 22, 2023

Codecov Report

Attention: Patch coverage is 20.94595% with 117 lines in your changes missing coverage. Please review.

Project coverage is 22.17%. Comparing base (d95bd1f) to head (81ad021).
Report is 143 commits behind head on main.

Files with missing lines Patch % Lines
pkg/sip/inbound.go 4.87% 39 Missing ⚠️
pkg/sip/outbound.go 0.00% 34 Missing ⚠️
pkg/sip/room.go 0.00% 32 Missing ⚠️
pkg/media/h264/h264.go 0.00% 10 Missing ⚠️
pkg/sip/signaling.go 93.54% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #35      +/-   ##
==========================================
+ Coverage   22.02%   22.17%   +0.15%     
==========================================
  Files          20       21       +1     
  Lines        1562     1655      +93     
==========================================
+ Hits          344      367      +23     
- Misses       1190     1260      +70     
  Partials       28       28              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iAbdulaziz93
Copy link

Hi @Hollymingyi. I'm working on supporting video calls over SIP on an application I'm working on, I'm using LiveKit. Did you get your issue solved? Is this PR is working fine on your side and the video issues like the video is fuzzing is solved on your end?

@iAbdulaziz93
Copy link

Hi @dennwc. Did you have a chance to review this PR? It will help us a lot if this PR gets reviewed and approved and added support of videos to LiveKit SIP.

@dennwc
Copy link
Contributor

dennwc commented Jun 6, 2024

Yes, I did, but at this point it needs to be updated to match the latest codebase.

And to get accepted, it would need to properly fallback to audio-only, which is not implemented in this PR.

Also note that it can only accept one video stream, while LiveKit room typically contain multiple participants. SIP currently mixes audio from multiple participants, so for proper support it must do speaker detection or something similar to switch video steam to the talking participant. Or at least fallback to audio-only if multiple participants are present.

This is the reason why we don't have plans to support it - it's adding complexity that could be avoided if client will run LiveKit in the first place. If the client is video-capable, it usually means it could probably run a browser with LiveKit and get all benefits of it without going through SIP.

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