Describe the bug
All images (posters, actors, etc) have hard coded sizes.
(240 脳 360 for posters)
Expected behavior
Images should have an acceptable resolution.
System (please complete the following information):
Issues go stale after 60d of inactivity. Mark the issue as fresh by adding a comment or commit. Stale issues close after an additional 7d of inactivity. If this issue is safe to close now please do so. If you have any questions you can reach us on Matrix or Social Media.
I think this is fixed in 10.4?
Depends on the screen, the sizes are still hardcoded.
Still an issue, images look grainy on every screen size (pretty much), when they don't when I look directly on TVDB, are we purposefully downsizing images for some reason?
I suppose this will definitely be fixed in 10.5.0, as I saw that my commits in 10.4 are still in master and no images have hardcoded sizes (the only one I found was the "logo" artwork of an item in the dashboard that appears when the item is being played in a device)
There is any other image that's hardcoded in size still? If that's the case, please tell me and I will change them. Otherwise, I think we can close this issue.
@ferferga unfortunately we will either have to revert this or add a configurable setting in the user preferences, because the full size images are causing performance issues in several situations.
@dkanada I'm a bit biased probably because I'm the author of those changes, but I think that we must diagnose why the images give performance issues in Android. Most webpages (and all modern browsers) I know load tons of huge images without issues (and you can even try yourself making a quick HTML page and loading more than 100 images, it will handle it without issues.
I'm also interested in making a plugin, and maybe I do one that optimizes images with jpegoptim and similar tools in an scheduled manner. But anyway, most of the images provided by TMDB (which is the main provider I use) are already losslessly optimized.
Remember also that we have a pending debug of #602, which is one of the most resource intensive scripts in the Web UI. Maybe we can remove it in Android if we can't find the issue in the 10.5.0 deadline? It's useless there.
Btw, I've been playing with master during this week and I can say for sure that the performance is much better than in stable release, so maybe that's already fixed but we don't know. Don't know when that happened either, lots of commits happened during this month and I've been out for a while.
@ferferga could you please also have a look into https://github.com/jellyfin/jellyfin/issues/2070
I still think it was caused by this PR https://github.com/jellyfin/jellyfin-web/pull/512
@dkanada Aside from the gamepadtokey fix, it seems to me that I would need to make a PR for adding a quality limit of 80 or 70 for reducing a bit the file size. Going down in resolution is crazy seeing how we're going towards bigger images through every part of the UI.
Would be a good idea to get feedback when the gamepadtokey.js fix is rolled, as for me, that solved all the performance issues in the UI alone.
@Nickbert7 already commented there
Well, I would rather we allow control of more variables (at least target quality and target size) for when you manipulate an image.
It's kinda pointless to make a preview item with quality=70 for a 8K-sized image IMHO.
@JustAMan An 8K image at quality 70 can be way way down of 500 KB, depending in color scheme, ranging a maximum of 1.2 MB in most complex scenes according to what I worked. Remember that 8K is just a 33 MP photo, which many phones nowadays take with qualities above 95 in most cases, with about 3 MB in size.
Probably I'm alone in this, but both when I used Jellyfin and Plex I always looked for the biggest image, in order to have the best possible quality in my library. If you're using 8K poster for images (at least nowaday), you're doing it because you want them in 8K.
Another option is to add a setting somewhere in config files. But if you're using big images, it's because you want them to look big (doesn't make sense otherwise) imo.
I still need to actually code this and test how it works. Will try with the biggest images I can find and take some conclusions.
I wasn't opposing that as much as you thought I did :) Was just saying that there's probably no fit-for-all value of quality threshold, so it should be configurable. 8K was just an illustrative example for that :)
@JustAMan Noooo I didn't mean that either, just wanted to explain a little better why I'm so "obsessed" with this matter (as I think I never explained it like this). Any point against the full-res images is valid because it's a fact that the bandwidth consumed and the loads time are a problems (which are important facts) and bigger resolutions are proportionally inverted to speed. So everyone is right in its own matter, imo. Gladly we have soooo many discussions for those little details (like the fonts) that might be irrelevant but still important
After clarifying that, yes, I completely agree with you. The good thing is that jpeg quality parameters scale really well, let me explain it: A 70 quality 720x480 image without zoom shouldn't look much different from an 8K one without zoom. The problems are always when we zoom.
Doing what you propose would require some kind of logic in the server code, meaning that we have two options:
If no one is interested in doing this client-side, I would need som help from someone in server for doing that, as, although I know something about C#, this codebase is pretty much over me and I'm still not that confident :).
I'm not sure that the work required will be profitable to be honest. Basically because of the "scale well" capabilities of jpegs and the fact that artifacts that appears in images with compression values that are considered to be the sweet spot (around 70) are only noticeable when you're seeing the image in its original resolution, while in Jellyfin most images will be displayed scaled down by the browser.
I believe we could use the <picture> element with multiple sources of different qualities to resolve most of these issues. I still need to verify that if the image sizes are causing the performance issues on Android.
At least for the images within the cards the cardBuilder had already calculated the size of the images by the count of cards per row https://github.com/jellyfin/jellyfin-web/blob/752212413ca702f198c26f5a3f0a009b8410a185/src/components/cardbuilder/cardBuilder.js#L17-L223 and the screen width https://github.com/jellyfin/jellyfin-web/blob/752212413ca702f198c26f5a3f0a009b8410a185/src/components/cardbuilder/cardBuilder.js#L238-L243
For single images like in the item detail page it is fine for me to keep the full resolution images.
Basically, my points are:
1) I don't think we should compress "original photos" to jpeg 70, they look bad when you look at them in detail (and looking in detail is the reason for keeping original photos IMHO)
2) I think we should make a distinction between "preview images" (like when there are dozens of them on a single screen - no one would probably zoom over single particular one) and "image as the media", like a photo being displayed. They should be processed differently, and I think we should compress (and probably downsize) preview images to save bandwidth and ease the browser work, but we shouldn't even _touch_ "image as media" images
We weren't planning on changing the original photos, just the copies sent to the clients.
This was also the new plan that @ferferga and @Nickbert7 mentioned so I think we are all on the same page now.
For the record, #746 fixed all the issues with the resource hog created by gamepadtokey.js
It still looks like some card lists aren't requesting smaller images, which isn't ideal. I can tell because the larger source images are taking much longer to load than the rest, but this shouldn't block the next release.
@dkanada Yeah, that's the deadline for closing this issue for good imo. @mrtimscampi and I discussed that the other day in Matrix (that we need to rework the original logic properly accordingly to newer standards and display sizes), however that will need to wait to 10.6 or even more. We aim to retain the exact same quality we have now, but with a reduction in bandwidth size and memory footprint.
I wanted to mention the merge of #746 as it was one of the first causes that we diagnosed that caused major issues with images, as I think that it is a good idea to have all of the things we do related to this tracked in this issue, so we can go back here and check what has been done and when.
Aside from #746, all the work done in Web for 10.5 by everybody in @jellyfin/web made this situation even better and we have now high quality images with a great performance (which was not the case for 10.4.3), so I think it's really safe to admit that we are pretty good to go in the current state.
The only thing we have ahead for this is the bandwidth consumption/memory footprint thing (and hopefully it doesn't take so long). But luckily, we're in great shape compared from how we started :D
2\. They should be processed differently
They already are, with a specific "getScaledImage" method in the API client.
This takes a maxWidth parameter (maybe more, need to look on the server side) that was removed to get images to look better in #512 .
My though for this is to improve on the initial way of doing things (computing a size for elements and passing it to getScaledImage.
Originally, it was getting the width of the window, and rounding it down to a set of fixed resolutions. It didn't take into account the way cover size works in CSS or different pixel scales.
My proposal is this:
This gives us a resolution to derive all the artwork size from, while keeping quality into account in all cases.
Closing this as it's fixed. Further improvements should go in separate issues.
Most helpful comment
They already are, with a specific "getScaledImage" method in the API client.
This takes a
maxWidthparameter (maybe more, need to look on the server side) that was removed to get images to look better in #512 .My though for this is to improve on the initial way of doing things (computing a size for elements and passing it to getScaledImage.
Originally, it was getting the width of the window, and rounding it down to a set of fixed resolutions. It didn't take into account the way cover size works in CSS or different pixel scales.
My proposal is this:
This gives us a resolution to derive all the artwork size from, while keeping quality into account in all cases.