motioneye-project
Fit vertically 2x2 as expected
Many thanks. Please try to avoid trailing spaces, indentation changes and code style changes, to keep the code changes better reviewable. I'll do some tests when I find time.
Yeah, my editor wanted to make use of indents/spaces consistent throughout this file so it changed the unrelated lines. I can override that so it's just changing lines relevant to this issue
I agree, a mix of landscape/portrait cameras complicates things. Maybe let's tackle that later (right now, I think releasing this would be better than the current state of affairs even if not covering 100% of use cases - we can follow up if users complain). Since CSS prevents having individual frames be different, follow ups would probably be: 1) making sure "combinedRatio" is OK as-is (which I think it is) and 2) apply a CSS class to each camera that inherits frame width or frame height, whichever makes sure that the cam fit into its frame regardless of its own aspect ratio >1 or <1 and whether its frame's aspect ratio >1 or <1
The UI probably shouldn't let you choose rows/columns that are inconsistent with the # of cameras chosen. Might have to fire an event when the user adds or deletes a camera, so that they adjust the rows/columns right then. There's no sensible default handling if you change # of cameras (you don't want to hide cameras, you don't want extra blank rows/columns with no cams)
Those points I raised were only things that popped up in my mind while reviewing the code, they might well be non-issues. At least what you wrote on them makes sense to me.
I agree that if by testing we can confirm that this PR generally improves things, even if leaving some earlier not-that-well handled edge cases dealt in a non-optimal way, it would make sense to merge. I don't even have any idea how to properly deal with great variations in the frame aspect ratios, other than maybe ordering them automatically so that similar frames are on the same row or column. But the users might anyway like to order the cameras themselves in their liking (and I think there's at least one issue for being able to change the order of the cams), so maybe there's not much use in trying to do anything about it here.
Pushed a new commit that reverts the unrelated whitespace change as discussed
Also noticed a regression relating to single-cam view not rendering correctly, so added a fix for it. @MichaIng does this look OK?