Adjust Paella scaling and background #984
Conversation
|
If I understand correctly, you're adjusting (maximizing) in line with the width available, correct? If so, do you also keep in mind that width should be set incrementally at 720, 1080, 1440 pixel (and higher numbers for UHD) so that the resolution is in line with HD video modes? Arbitrary widths would lead to lower quality AFAIK. |
|
PS: With my primary monitor at 1920x1080 (viewport 1280 × 579 ) the outcome is not very favorable: |
|
Hm I don't think this affects resolution or video size at all. That is still being handled by paella itself. For comparison: I do agree however that on your first screenshot, the playback bar is taking up too much vertical space of the video. Right now I'm uncertain whether that is something that we can fix in Tobira. |
|
So this issue is "only" about scaling the control bar in the way you described? |
You are correct in principle: if the video resolution does not match the output resolution exactly, it needs to be scaled (up or down), which reduces sharpness slightly. However, this is a very small effect that is fully overshadowed by things like video compression. And additionally, having a web app make sure that the video resolution matches the output resolution exactly is not feasible. Hence, in Tobira we can use arbitrary video widths in order to optimize the layout on the page.
Can you describe, what exactly you dislike about that? Is it the fact that (as Ole said), the control bar takes up a lot of vertical space and thus obstructs the video. Or is it the empty space left and right of the whole video container? Or is it that you would rather have the video be taller, i.e. take more vertical (and consequently also horizontal) space? |
owi92
force-pushed
the
paella-scaling-and-background
branch
from
a148c03
to
c17ddb8
Compare
The two last things are what I dislike: Lots of empty space to the left and to the right, so yes, the video should be taller in both dimensions. Caveat: My primary display seems to work with scaling, so maybe that's not something others will come across. |
|
PS: Looks better on my MacBook Pro, but still some space unused: |
This has been like this for quite a while now. Like I said before, this PR only removes the grey bars on both sides of the video, like we discussed in #954. We can really only make the video larger by either
Do you prefer any of these options? |
|
I think removing the breadcrumbs is not a good idea: having them in one place consistently makes for a good UX. Moving the title a bit further down is something we can do I think. I would still make the title at least mostly visible, but the description box and date doesn't need to be visible IMO. Further, I could imagine sliiightly decreasing the margin above and below the player. Specifically (mostly for Ole):
I think these small changes could already make a big difference. We could also increase the min-height (currently 180) a bit. This means that the title will be pushed out of the screen "earlier", i.e. already on not so small screens. |
This unifies the colors of video and preview image letterboxing as well as the canvas for dual stream videos.
One does not simply set this player's width. So this uses a rather roundabout way of doing it. First, the "scaling" is deduced by comparing the actual factual video height (`offsetHeight)`, which is then used to calculate the video width. Why not use the `offsetWidth` in the first place? Well... setting the width of something by comparing it to itself can lead to certain issues. This also uses a `resizeObserver` to dynamically update the ui width, because why not.
owi92
force-pushed
the
paella-scaling-and-background
branch
from
c17ddb8
to
5fd0b03
Compare
frontend/src/ui/player/index.tsx
Outdated
| maxHeight: "calc(100vh - var(--header-height) - 18px - 16px - 42px - 120px)", | ||
| minHeight: 180, | ||
| maxHeight: "calc(100vh - var(--header-height) - 18px - 16px - 38px - 60px)", | ||
| minHeight: 320, |
There was a problem hiding this comment.
Unfortunately, that now breaks for small screens. I think this works:
minHeight: `min(320px, (100vw - 32px) / (${aspectRatio[0]} / ${aspectRatio[1]}))`,
It's a bit weird but yeah. The more straight forward way would be to set maxWidth: 100% but that does not lead to the desired outcome (due to conflicting requirements the browser gets confused I think). When setting aspect-ratio its best to only restrict one dimension. And well we can simplify calculate the corresponding height by using the aspect ratio. The -32px is due to the page padding btw. I thought I would need to remove that for the "very narrow screen" when that padding does not exist anymore. But that doesn't seem to be a problem for some reason.
There was a problem hiding this comment.
Oh right, I thought I tested this but I must have mixed things up. Thank you for the suggestions, they work pretty well.
|
Looks slightly better / acceptable on my primary monitor now:
But feel free to ignore that particular device and make it work at your end(s). |
owi92
force-pushed
the
paella-scaling-and-background
branch
from
8b4e441
to
292d686
Compare
Closes #954
This is.. the best I could come up with. Might seem weird to infer the initial video width using it's height and resolution but I don't see a better way.
I was wondering if this needs a special handling for multi-stream videos but couldn't make out a difference. Even though there is more space, the individual streams don't really care to use it (related: #956).