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

Remove RTMP streams for nvr connected cams #1650

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

apocaliss92
Copy link
Contributor

No description provided.

streams.push(rtmpSub, rtspMain, rtspSub);
const isNvr = this.client.getIsNvr(deviceInfo);

if (isNvr) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting comparing, this is the only block added here (708-719), the non nvr branch is not changed at all

@koush
Copy link
Owner

koush commented Nov 27, 2024

is there a better way to do this, to query if rtmp/rtsp is available somehow.

@apocaliss92
Copy link
Contributor Author

Actually the streams are available but are broken when thorough NVR, we guided 2 users already this week to switch from rtmp to rtsp, maybe would just be better to set the defaults to rtsp? In that case what would be the criteria to let Scrypted pick them instead of rtmp, just order in the streams array?

@koush
Copy link
Owner

koush commented Nov 27, 2024

Actually the streams are available but are broken when thorough NVR, we guided 2 users already this week to switch from rtmp to rtsp, maybe would just be better to set the defaults to rtsp? In that case what would be the criteria to let Scrypted pick them instead of rtmp, just order in the streams array?

The issue with RTSP is that it is buggy on many reolink cameras causing dropouts or camera crashes. Furthermore RTSP only offers 2 streams and not 3 like RTMP.

@koush koush force-pushed the main branch 2 times, most recently from 561ab2c to 9b4547b Compare December 6, 2024 19:14
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.

2 participants