Improve image loading speed and sizes

[heyhippari]

Changes

Improves loading times and image sizes for most cards, backdrops and the details page.

Issues

Fixes #911
Fixes #858

Edit: I'm marking items done with 👍

[dmitrylyzo]

Person cards are limited with 160px - looks ugly on TV.

jellyfin-web/src/controllers/itemdetailpage.js

Line 1862 in 9a4d992

width: 160,

On Desktop, person card width is 182px, but again image is limited by 160px

[dmitrylyzo]

Removed that string - much better: requests 446px.

[heyhippari]

This needs to have videoosd.js looked at before any merge (As reported by JustJane on Matrix)

@jellyfin/web If you find more examples of pages that are super slow, please mention it here.

Already included at this point:

• Details page
• Everything that uses CardBuilder.js

[Nickbert7]

I'm not sure if it is already covered, but the images within the libraries settings are not scaled.

[heyhippari]

Playlist thumbnails also need a pass.

[dmitrylyzo]

"Active Devices" in Dashboard

[ferferga]

This needs to have videoosd.js looked at before any merge (As reported by JustJane on Matrix)

@jellyfin/web If you find more examples of pages that are super slow, please mention it here.

Already included at this point:

* Details page

* Everything that uses CardBuilder.js

@MrTimscampi I need to deeply check everything that re-scales and test your branch, so I'm not sure if it's affected by CardBuilder, but I think that posters of the elements in the itemdetails page should be at full resolution, no matter what. Ignore me if they are.

I'm also wondering what's your solution for window re-scaling. What if I open the window side by side another one and then, I max it out? Images will be blurried?

PS: Taking a quick look over the diffs, I saw that you're using window sizes to calculate that, which is the wrong approach imo because of the re-scaling of the window thing I just commented.

[heyhippari]

I think that posters of the elements in the item details page should be at full resolution, no matter what

I respectfully disagree, because images can be MASSIVE. For example, my poster for Blade Runner 2049 is 3000x2000 and is 1MB in size. This is massive, especially on a mobile connection where data is at a premium. Even on a desktop, it significantly increases page size, which means longer loading times. We've already had a few users that have noted the negative change.

And not only does it make pages heavier, but it also creates significant aliasing, which simply looks bad.

I'm also wondering what's your solution for window re-scaling

Resizing happens so rarely that it's honestly not worth it to optimize for that case, in my opinion. It would mean reloading all the images as soon as a resize happens, which, again, would generate a ton of data usage, when people will likely navigate away from the page.

What if I open the window side by side another one and then, I max it out?

In this specific case, your window would be taller than it is wide, so we recalculate the full width of your window in portrait mode at a 16:9 resolution (Chosen because it's the most common for artwork). Essentially, you'd get the same backdrops with two windows side-by-side than you would with one window maximized. Cards artwork size is usually computed based on their display size, but we have 2x the size displayed as a margin if the user resizes the window, which should be more than enough.

---

The issue is that it's a difficult balance to strike. We need to keep both artwork quality AND performance/data usage in mind. We cannot get rid of one for the other, hence the solution chosen here to have most artwork be 2x their resolution on the screen, in order to sidestep Skia's bad rescaling while ensuring that images don't have an enormous resolution unless they need it.

[dmitrylyzo]

@MrTimscampi I think float numbers are not supported. At current moment, all is fine, but maybe round them just for sure.

[heyhippari]

@dmitrylyzo Good point! Done ;)

[dmitrylyzo]

@MrTimscampi Sorry, I guess I misled you. 😅

jellyfin-web/src/libraries/apiclient/apiclientcore.js

Lines 220 to 223 in 76b0859

	function normalizeImageOptions(instance, options) {
	var ratio = instance._devicePixelRatio || 1;
	ratio && (options.minScale && (ratio = Math.max(options.minScale, ratio)), options.width && (options.width = Math.round(options.width * ratio)), options.height && (options.height = Math.round(options.height * ratio)), options.maxWidth && (options.maxWidth = Math.round(options.maxWidth * ratio)), options.maxHeight && (options.maxHeight = Math.round(options.maxHeight * ratio))), options.quality = options.quality || instance.getDefaultImageQuality(options.type), instance.normalizeImageOptions && instance.normalizeImageOptions(options)
	}

And it looks like devicePixelRatio is also taken into account (but it is not dynamic).

[heyhippari]

@dmitrylyzo Hm, interesting.

Should we remove it from the API Client and handle it for each request? (Would be better if the user changes the window)

We can also remove it from all individual requests and make the one in ApiClient dynamic.

[dmitrylyzo]

Should we remove it from the API Client and handle it for each request? (Would be better if the user changes the window)

Thinking about possible purpose, having it in ApiClient might be useful for forced pixel ratio. If we ever need this.

We can also remove it from all individual requests and make the one in ApiClient dynamic.

I have no argument against. This will make the rest of the code simpler and pixel ratio will be in single place. Dunno if someone wants pixel ratio not for images.

At current moment, we have devicePixelRatio at:

• your dom.getScreenWidth
• bunch of maxWidth/maxHeight
• cardbuilder but it is unused

  jellyfin-web/src/components/cardbuilder/cardBuilder.js

  Line 5 in 76b0859

  var devicePixelRatio = window.devicePixelRatio || 1;

[dmitrylyzo]

Music / Suggestions / RecentlyPlayed and Music / Suggestions / TopPlayed are not limited
width become null: primaryImageAspectRatio is undefined
https://github.com/MrTimscampi/jellyfin-web/blob/039728359914d18765a78f22b2aaf73fc9a1aebf/src/components/cardbuilder/cardBuilder.js#L593-L604

[ferferga]

I respectfully disagree, because images can be MASSIVE. For example, my poster for Blade Runner 2049 is 3000x2000 and is 1MB in size. This is massive, especially on a mobile connection where data is at a premium. Even on a desktop, it significantly increases page size, which means longer loading times. We've already had a few users that have noted the negative change.

The negative change is obviously noticeable when there are more than 10 images that we're loading at once. But in the new detail's layout, poster images are the heart of it, they're no longer of the same size as the cards in the library view as it used to be in 10.4.3.
The same applies to backdrops.

I don't see any issues loading 1 MB images, that's the usual size of images that are that big in webpages.

That for Desktop and TV, of course mobile can have some constraints (as that's where the data caps are really).

Resizing happens so rarely that it's honestly not worth it to optimize for that case, in my opinion. It would mean reloading all the images as soon as a resize happens, which, again, would generate a ton of data usage, when people will likely navigate away from the page.

That is one of my main workflows, where I have something in the left and Jellyfin's playlist or TV Show going on. I don't see any reason why anybody doesn't use it that way, I'm sure that I'm not that alone in this.

In this specific case, your window would be taller than it is wide, so we recalculate the full width of your window in portrait mode at a 16:9 resolution (Chosen because it's the most common for artwork). Essentially, you'd get the same backdrops with two windows side-by-side than you would with one window maximized. Cards artwork size is usually computed based on their display size, but we have 2x the size displayed as a margin if the user resizes the window, which should be more than enough.

Okay, that's great, thanks for clarifying! However, this couldn't be solved ignoring completely the size of the window and taking into account the screen size?

The issue is that it's a difficult balance to strike. We need to keep both artwork quality AND performance/data usage in mind. We cannot get rid of one for the other, hence the solution chosen here to have most artwork be 2x their resolution on the screen, in order to sidestep Skia's bad rescaling while ensuring that images don't have an enormous resolution unless they need it.

But, as said above, it's way too overkill to simply do it everywhere (as it was an overkill doing full everywhere). We should mix both in my opinion, just like I mentioned above: Backdrops, Poster images in item details and profile pics should be at max res imo.

I didn't say that before, but resizing profile photos is probably one of the worst ideas, at least how it was implemented before. It's better to download the full res image once and rely in browser's cache everywhere (just like the current implementation) that generating multiple sizes that can change everywhere, across one session in fact.

[ferferga]

I've been trying your branch, and the only thing I can say is thank you very much for achieving this balance. It's impressive how it retains the quality while loading fast. You didn't try Jellyfin before I made the full-res PR, right? You will understand why this is so important:

However, I still don't agree with the re-scaling of the images in details section:

Scale

After

Although it still looks great and I agree this is fine for 95% of the people, it's not for the 5% of us who are so focused in the tiny details. Notice how, in the scaled version, Pennywise's face is almost unnoticeable in the globe. Blood splatters in the name are also a blurried mess. And that's without diving more in other aspects of the image.

Again, that can be good for a layout where the poster takes 50% less of space, but in this layout, it's the heart of everything. And this has been tested in Desktop with relatively high DPI, not sure what would happen with large screens with little resolution, didn't had the time to test it yet.

It's not that much of a problem as I told before, because 95% of the users will be happy about it. However, it's a single image and, in fact, I feel that re-scaling the main "showcase" for the poster would simply make choosing higher resolution in the image editor absolutely worthless.

Btw, would be great as well if @jellyfin/backend can do something with Skia in their 10.6 roadmap. But for client, we reached a fairly nice state. Very good work and thanks for your knowledge in this! @MrTimscampi

[dmitrylyzo]

Looked again why Music / Suggestions / RecentlyPlayed and Music / Suggestions / TopPlayed are unlimited. Only first item has PrimaryImageAspectRatio field - something wrong on server side.

At least we could make this part the same as the others.

--- a/src/components/cardbuilder/cardBuilder.js	2020-03-09 22:55:31.460475765 +0300
+++ b/src/components/cardbuilder/cardBuilder.js	2020-03-10 20:13:57.060552674 +0300
@@ -592,7 +592,7 @@
                 });
             } else if (item.AlbumId && item.AlbumPrimaryImageTag) {
 
-                width = primaryImageAspectRatio ? Math.round(height * primaryImageAspectRatio) : null;
+                height = width && primaryImageAspectRatio ? Math.round(width / primaryImageAspectRatio) : null;
 
                 imgUrl = apiClient.getScaledImageUrl(item.AlbumId, {
                     type: "Primary",

I stopped liking a lot of hardcoded things

[dmitrylyzo]

Another one

jellyfin-web/src/components/remotecontrol/remotecontrol.js

Lines 124 to 128 in 291f0b1

	var url = item ? seriesImageUrl(item, {
	maxHeight: 300
	}) || imageUrl(item, {
	maxHeight: 300
	}) : null;

[anthonylavado]

/azp run

[azure-pipelines]

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

[heyhippari]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[dkanada]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).