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

feat: fullscreen camera page #609

Merged
merged 12 commits into from
Jun 16, 2022

Conversation

matmen
Copy link
Member

@matmen matmen commented Mar 24, 2022

Adds a dedicated page for the fullscreen camera feed, solving the following issues:

  • Camera settings are now respected (rotation, fps, ...)
  • The emergency stop button (as well as all other menu items) are now available while being in the full screen view

Currently still opens in a new tab, not sure if this is the appropriate behavior

Tested with (adaptive and streamed) MJPEG sources on:

  • Chrome Desktop
  • Chrome Mobile (both landscape and portrait orentation)
  • Firefox Desktop
  • Edge Desktop

image

Closes #542 #279

Signed-off-by: Mathis Mensing <[email protected]>
@matmen matmen added FR - Enhancement New feature or request UI - Cams Everything related to cameras in fluidd. labels Mar 24, 2022
@matmen matmen added this to the 1.18 milestone Mar 24, 2022
@matmen matmen requested a review from pedrolamas March 24, 2022 21:54
@matmen matmen linked an issue Mar 24, 2022 that may be closed by this pull request
@matthewlloyd
Copy link
Contributor

How does FPS compare against a simple stream tab, when using higher resolution or higher FPS cameras? Please see my comments on UI performance here: #525 (comment)

I think my comments from the full-screen console card may be worth bearing in mind here also: #613 (comment)

Could we make this behavior optional via a setting, or introduce an additional button for the original direct camera feed? Personally I will always prefer to have a simple direct stream opened in a tab, and I think many other users will too given that very few people are likely using the hflip/vflip options.

I think a good compromise that makes everyone happy might be as follows:

  • Add a full-screen webcam nav bar item, with a full-screen view that is essentially just a big version of the existing dashboard card, showing all webcams by default but with the ability to select an individual webcam for a larger view.
  • Add a full-screen icon (diagonal arrows) to the webcam dashboard card, which is a shortcut for the webcam nav bar item.
  • Keep the existing "raw stream in tab" button (perhaps with a setting to show or hide?) so that people who want a quick way to access the raw stream don't lose the ability to do so.

@matmen
Copy link
Member Author

matmen commented Mar 26, 2022

How does FPS compare against a simple stream tab, when using higher resolution or higher FPS cameras? Please see my comments on UI performance here: #525 (comment)

I'm streaming my main camera at 1080p@30fps using adaptive MJPEG, and I'm getting a stable 30fps in the full screen view with this. The iframe option gives me around 60fps too. I can only see fps limiting happening on the dashboard and it looks to be because of the notifyStatusUpdate dispatching/handling.

Could we make this behavior optional via a setting, or introduce an additional button for the original direct camera feed? Personally I will always prefer to have a simple direct stream opened in a tab, and I think many other users will too given that very few people are likely using the hflip/vflip options.

I don't really think this is an uncommon issue - from what I've seen on the Discord a lot of people are having issues with the fullscreen view not taking into account rotation and flipping. The only way for them to solve that issue would be editing the webcam config, which isn't trivial (at least much less than clicking a UI dropdown).
I'm fine with keeping the original stream URL (or adding a toggle to the settings), though.

I think a good compromise that makes everyone happy might be as follows:

  • Add a full-screen webcam nav bar item, with a full-screen view that is essentially just a big version of the existing dashboard card, showing all webcams by default but with the ability to select an individual webcam for a larger view.
  • Add a full-screen icon (diagonal arrows) to the webcam dashboard card, which is a shortcut for the webcam nav bar item.
  • Keep the existing "raw stream in tab" button (perhaps with a setting to show or hide?) so that people who want a quick way to access the raw stream don't lose the ability to do so.

This seems fine to me. Would love to get some input from @pedrolamas (and maybe @Durahl and @PigeonFX too) on this too.

@matthewlloyd
Copy link
Contributor

I'm streaming my main camera at 1080p@30fps using adaptive MJPEG, and I'm getting a stable 30fps in the full screen view with this. The iframe option gives me around 60fps too. I can only see fps limiting happening on the dashboard and it looks to be because of the notifyStatusUpdate dispatching/handling.

Sounds good, thanks for confirming!

I don't really think this is an uncommon issue

I suspect the majority (90%?) aren't using those options so they would enjoy the raw full-screen tab, and I definitely agree enough people are using rotation and flip for it to be important to provide for those users too.

I'm fine with keeping the original stream URL (or adding a toggle to the settings), though.

Sounds good. Can we have both please! :D Seems like we are in agreement. I can see myself using both a full-screen webcam nav bar item which shows both cameras at once, and also the raw stream URL in a new tab option, depending on what I'm trying to see.

@matmen matmen marked this pull request as draft March 26, 2022 20:42
@matmen
Copy link
Member Author

matmen commented Apr 18, 2022

@matthewlloyd I'm not really happy with all the implementations involving just slapping the camera card onto its own page.. The cameras end up being too small and the button to change cameras causes a global state change to happen, which causes weird UX behavior on the dashboard. From the suggestion (#279 (comment)) I think just showing a single camera at a time (per page) would be fine, what do you think? I've pushed something I've had stashed for the better part of 3 weeks now, but I'm not sure if I'm happy with it lol

@matmen matmen modified the milestones: 1.18, 1.18.1 Apr 27, 2022
@matmen matmen modified the milestones: 1.18.1, 1.18.2 May 20, 2022
@matmen
Copy link
Member Author

matmen commented May 26, 2022

I went back and tried some alternative ways to solve this today, and so far I wasn't happy with any of them (UX wise, my PR included).. If anyone has specific suggestions I'm happy to implement them

@pedrolamas pedrolamas modified the milestones: 1.18.2, 1.19 Jun 9, 2022
@matmen matmen marked this pull request as ready for review June 14, 2022 11:25
@matmen
Copy link
Member Author

matmen commented Jun 14, 2022

Marking this as ready for review as I haven't received more feedback yet, maybe this is worth being an intermediate solution ;)

pedrolamas
pedrolamas previously approved these changes Jun 15, 2022
@matmen matmen merged commit 9a159ec into fluidd-core:develop Jun 16, 2022
@matmen matmen deleted the feat/fullscreen-camera-page branch June 16, 2022 07:25
matmen added a commit to matmen/fluidd that referenced this pull request Jun 27, 2022
Signed-off-by: Mathis Mensing <[email protected]>
Co-authored-by: Pedro Lamas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FR - Enhancement New feature or request UI - Cams Everything related to cameras in fluidd.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fullscreen Camera View not respecting Settings Full Screen webcam feed
3 participants