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.

Sign up for GitHub

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

Jump to bottom

feat: add WebRTC (MediaMTX / rtsp-simple-server) webcam mode #1318

Merged
merged 18 commits into from
Jun 18, 2023
Merged

feat: add WebRTC (MediaMTX / rtsp-simple-server) webcam mode #1318

merged 18 commits into from
Jun 18, 2023

Conversation

slynn1324
Copy link
Contributor

Adds support for ws://... urls in the IP Camera webcam type.

See #1317.

@meteyou
Copy link
Member

meteyou commented Mar 21, 2023

Thx for you your PR! The main problem with WebRTC is that there are so many different "handshakes" for WebRTC. It's not possible to have one webrtc mode to support all streamers.

I only would prefer to have a separate mode for this webcam style. It looks like the "wyze-bridge" is using the rtsp-simple-server in the background for the webrtc stream. So I think the name "WebRTC (rtsp-simple-server)" would be a good name for this mode.

The mode "IP-Webcam" is for generic IP-Webcams.

@slynn1324
Copy link
Contributor Author

Fair enough... I didn't dig into all the details of the underlying protocols (just hacked together what worked from an example) but that makes sense. I'll look at refactoring this out to a separate camera type and update this PR. Thanks!

@github-actions
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@slynn1324
Copy link
Contributor Author

Updated as a new camera type. I think I poked around and found the necessary references that the camera types need expanded. Please take a look.

meteyou
meteyou reviewed Mar 22, 2023
View reviewed changes
Copy link
Member

@meteyou meteyou left a comment

Choose a reason for hiding this comment

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

in general, this already looks very good. i added a few comments, but after that it should fit. i will try to install this streamer to be able to test this.

@meteyou meteyou added this to the v2.6.0 milestone Mar 23, 2023
@slynn1324
Cleam up WebrtcRTSPSimpleServer webcam, don't monitor visibility for …
0b9625e 
…playback but instead monitor panel collapse and component destroy.
@github-actions
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@github-actions
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@github-actions
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@slynn1324
Copy link
Contributor Author

See how that is. I reworked the start/stop logic to track when the panel is actively collapsed vs. just scrolled out of view. I also had to make a small change to TheSettingsMenu.vue so that the component would be destroyed if you click on the 'x' to close to the settings dialog without either cancelling or updating the camera in the dialog first. I think this strikes a good balance between not reconnecting too frequently yet only keeping the streaming connection open if it stands to be displayed.

I think I addressed all of the other comments.

Thanks.

@meteyou meteyou changed the title WebRTC IP Camera Support (Issue #1317) feat: add WebRTC (rtsp-simple-server) webcam mode (Issue #1317) Apr 1, 2023
@meteyou meteyou changed the title feat: add WebRTC (rtsp-simple-server) webcam mode (Issue #1317) feat: add WebRTC (rtsp-simple-server) webcam mode Apr 1, 2023
@meteyou
refactor: simplify some code blocks
efa7446 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@meteyou
refactor: change trimLeft to trimStart
30106dd 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@meteyou
feat: add webrtc status output
6b99ef5 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2023

Language file analysis report:

File Missing Keys Unused Keys
de.json 1 0
en.json 0 0

@meteyou
refactor: rename WebRTC (rtsp-simple-server) to WebRTC (MediaMTX)
4c5e33b 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@meteyou
refactor: rename WebRTC (rtsp-simple-server) to WebRTC (MediaMTX)
932b3b7 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2023

Language file analysis report:

File Missing Keys Unused Keys
de.json 1 0
en.json 0 0

@meteyou
locale(de): add german locale
d57b90c 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@meteyou meteyou changed the title feat: add WebRTC (rtsp-simple-server) webcam mode feat: add WebRTC (MediaMTX / rtsp-simple-server) webcam mode Apr 2, 2023
@meteyou
Copy link
Member

meteyou commented Apr 2, 2023

I cleaned the file and added TS types as well as possible. Yesterday also, the project rtsp-simple-server was renamed to MediaMTX. So I also renamed it in this PR. I tested it with a newly installed MediaMTX with a raspberry webcam. Please double-check if it also works in your case, and then this PR will be merge-ready.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2023

Language file analysis report:

File Missing Keys Unused Keys
de.json 1 0
en.json 0 0

@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2023

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

@slynn1324
Copy link
Contributor Author

Apologies for the delay in getting back here. I pulled the updates and installed them and it seems to work fine with my setup here and the wyze-bridge stream.

I did notice 2 things however that were working better (in my opinion) prior to your updates:

  1. The web socket connection would not be opened when the panel was not expanded. The way that the panels seem to work on the dashboard (presumably due to animations), the components inside are mounted even when they are not visible. I do not think that the additional load on the backing servers (or the bandwidth) should be used to open the stream if it is not visible.

  2. The controls attribute was removed from the

I typically have mainsail up on 3 devices simultaneously: A monitor by the printer -- no camera needed there. I keep the Webcam panel minimized), my main PC in my office where I do most of my work, but monitor the printer from another room in my house, and then checking in from iPhone or iPad devices when I'm not in one of those places.

Of the two issues, I think #1 should be looked at more closely and corrected for to be kind to users bandwidth and device needs, and #2 is more of a preference.

@meteyou
Copy link
Member

meteyou commented Apr 16, 2023

@slynn1324 thx for your high-level feedback! You are right about point 1. I saw it as "unimportant" when refactoring and wanted to remove unnecessary complexity, but with your explanation of your workflow, this sounds very reasonable! Do you want to add this back, or should I do it?

I intentionally removed the controls because not all functions work and would only lead to "wrong bug reports". I did the same with the WebRTC (camera-streamer) client. But I wouldn't know how to hide specific controls, so I removed them all. We can gladly enable them again if you know how to show and hide them to ensure 100% compatibility.

@slynn1324
Copy link
Contributor Author

If you want to add back #1 as you'd like it to be that'd be great. It might be a bit before I'm able to get back into the code.

As for the controls - I would doubt there is a way to use only some of the native controls. On my iOS devices, it shows up with the native player controls which then allows full screen and pausing etc. It also displays that it's a live stream. Would this be something to consider an extra option in the webcam configuration panel about whether the native controls should be displayed or not? I could really see different users wanting it different ways... depending on their preferences.

@meteyou
refactor: start stream only when panel is open
f5dc635 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@meteyou
refactor: validate URL before connecting to websocket
a981532 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented May 1, 2023

Language file analysis report:

File Missing Keys Unused Keys
de.json 0 0
en.json 0 0

@meteyou
Copy link
Member

meteyou commented May 1, 2023

@slynn1324 i pushed a few changes to fix the closed panel behavior. it was included in the code, but with a bug.

Please test the build again.

@slynn1324
Copy link
Contributor Author

Tested here and seems to be working fine. I would think this is good to merge whenever you're ready.

meteyou
meteyou reviewed May 8, 2023
View reviewed changes
src/components/panels/WebcamPanel.vue Outdated Show resolved Hide resolved
src/components/webcams/WebrtcMediaMTX.vue Outdated Show resolved Hide resolved
src/components/TheSettingsMenu.vue Outdated Show resolved Hide resolved
@meteyou
Copy link
Member

meteyou commented May 8, 2023

Ok. I reviewed the code in detail and have found some little refactorings and one question. But the rest looks good to me. We also have to update this branch before merging it because other webcam clients were added to the developer branch, and there are merge conflicts right now.

@meteyou
Copy link
Member

meteyou commented Jun 15, 2023

@slynn1324 any updates here?

@meteyou
Merge branch 'develop' into feature/add-ip-webcam-webrtc
69392e9 
# Conflicts:
#	src/components/panels/WebcamPanel.vue
#	src/components/settings/SettingsWebcamsTab.vue
#	src/components/webcams/WebcamGrid.vue
@meteyou
refactor: add webrtc-mediaMTX to webcam-wrapper
487cd0b 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@meteyou
refactor: remove unused prop
3996f1a 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@github-actions
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 17 1
en.json 0 1

@meteyou
refactor: remove v-if="showSettings" in TheSettingsMenu component
cf74c60 
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@github-actions
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
de.json 17 1
en.json 0 1

@meteyou meteyou merged commit 8682dd7 into mainsail-crew:develop Jun 18, 2023
9 checks passed
@meteyou meteyou linked an issue Jun 19, 2023 that may be closed by this pull request
Add support for webrtc streaming for IP Cameras #1317
Closed
@meteyou
Copy link
Member

meteyou commented Jul 27, 2023

@slynn1324 can you share any information how to setup this webcam in wyze-bridge and which url you need to use in mainsail?

@Naninani
Copy link

@meteyou @slynn1324 Could you share instructions on how to setup?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@meteyou meteyou meteyou approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.6.0
Development

Successfully merging this pull request may close these issues.

Add support for webrtc streaming for IP Cameras
3 participants
@slynn1324 @meteyou @Naninani