Thumbnails size in file list doesn't match requested preview size

[bazuchan]

Steps to reproduce

Open folder containing images. It shows thumbnails in 64x64 size. But requests images of size 500x500 to display this thumbnails.

Expected behaviour

Images of size 64x64 are requested to display 64x64 thumbnails.

Actual behaviour

Images of size 500x500 are requested to display 64x64 thumbnails.

Server configuration

Operating system:
Ubuntu 18.10

Web server:
nginx + php-fpm

Database:
mysql

PHP version:
7.2

Nextcloud version: (see Nextcloud admin page)
15.0.2

Updated from an older Nextcloud/ownCloud or fresh install:
Updated

[skjnldsv]

Hello! This is on purpose for a compatibility with the grid view :)
We had a discussion about it (performance wise) and decided it was still very decent.

[bazuchan]

(performance wise) and decided it was still very decent.

Well, it is not for me. I can't scroll folder containing several thousand of images with 500x500 previews, it will stop showing previews at all after showing around 50-100 first ones (looks like because server timings to get image gets up to 10 seconds). And I do have pregenerated 512x512 previews. If I change to 64x64 previews scroling works fine. This is on 8-core VM on top of 2xE5-2620v1 server.

Also, 500x500 previews are 50-100kb in size, just loading folder with 100 images requires ~10MB of data transfered.

[skjnldsv]

Actually we fetch 250px images and they're about 20KB

[bazuchan]

I see https://xxx/index.php/core/preview?fileId=332354&c=1b49252159ee603963dd872140f8ab11&x=500&y=500&forceIcon=0 urls requested in developer tools.

[skjnldsv]

server/apps/files/templates/list.php

Line 34 in 6a3f4e4

<table id="filestable" class="list-container <?php p($_['showgridview'] ? 'view-grid' : '') ?>" data-allow-public-upload="<?php p($_['publicUploadEnabled'])?>" data-preview-x="250" data-preview-y="250">

data-preview-x="250" data-preview-y="250" :)

[bazuchan]

server/apps/files/js/filelist.js

Line 2106 in 0c8a000

urlSpec.x *= window.devicePixelRatio;

and I do have HiDPI monitor both on laptop and mobile.

[skjnldsv]

Nice catch! I guess it's then a bit too much! Thanks :)
@jancborchardt shall we revert?

[jancborchardt]

This is the first report we get about this, so not sure it warrants reverting?

Also, at this stage the grid view is not default yet, but it might be with Nextcloud 17 if we polish it more. And then we need the big thumbnails anyway. So I would say we should leave it as is.

[dgtlmoon]

I can confirm this at current 17.0.1 master my head is 51d3f98bfb83b632be6223a5d928d57e8e52e942

The thumbnail according to chrome inspector is 150x150 pixels.

the background url is http://172.18.0.1/core/preview?fileId=446&c=80c49cc440c54e05493e75f064aaa36e&x=325&y=325&forceIcon=0&asdfasd=adsfasdf23

however the response gives me a 1024x1024 image, considering i may have thousands - really saturating my network IO

1024-1024-crop.jpg: JPEG image data, JFIF standard 1.01, resolution (DPI), density 96x96, segment length 16, comment: "CREATOR: gd-jpeg v1.0 (using IJG JPEG v62), quality = 90", baseline, precision 8, 1024x1024, frames 3

Digging deeper I , when I access that URL see that Generator\getPreview() $width and $height are called with a value of 325 but when it reaches

		// Calculate the preview size
		list($width, $height) = $this->calculateSize($width, $height, $crop, $mode, $maxWidth, $maxHeight);

$width and $height become 1024 inside of calculateSize()

because in ce10f8b

This resulted still in a lot of possible images. Generating a lot of
server load and taking up a lot of space.
This moves it to previews to be powers of 4: 64, 256, 1024 and 4096
Also the first two powers are always skipped (4, 16) as it doesn't make
sense to generate previews for that.
We cache preview pretty agressively and I feel this is a better
tradeoff.

essentially - it gets rounded up to 1024 because it is not 325 and not near 256

In other words - whatever is setting the thubnail size preference is TOTALLY incompatible with the backend's ability to use scaled image sizing, the backend is limiting to a select size of thumbnails! either 256 or 1024 !!

I set in config.php ..

  'preview_max_x' => 325,
  'preview_max_y' => 325

but

		list($maxWidth, $maxHeight) = $this->getPreviewSize($maxPreview, $previewVersion);

still returns 4092, 3072

so the thumbnail sizing is a big steamy pile :/

.... This moves it to previews to be powers of 4: 64, 256, 1024 and 4096

at ce10f8b it is changed from rounding to a size of powers of 2 to powers of 4...

reverting that commit atleast gives me a 245 pixel image, instead of a 1024 image, although it's still loaded in a 130x130 pixel box......

in Drupal and Wordpress you just define an image size for the thumbnails, and then it's created once when required. no one cares if they get a slightly lower or slightly higher resolution thumbnail.

Whatever is doing the fancy JS for detecting DPI etc and then requesting a certain size thumbnail , in my opinion is a total waste of time, it should just dish up a general thumbnail FAST, this would entirely remove the all this wierd code about setting power of 2 or power of 4 pixel sized limit images, cut down on CPU and Disk IO just use a default size.

then on image request just get the one size.

there, fast, and you have less data to worry about

https://github.com/nextcloud/server/blame/0c8a0007a98d4b8df4b53298451d822292605be6/apps/files/js/filelist.js#L2106

this concept of caring about the DPI for thumbnails is 6 years old! I really think it needs to be thrown out, no one cares about getting exactly an amazing thumbnail at the tradeoff of having dozens of open issues here on github about slow thumbnail generation because it's making 6 useless thumbnails per image

• remove the DPI stuff for thumbnails, I have never seen this in other systems, it adds too much complexity and dependencies on different thumbnail sizes
• have a default thumbnail size which is clearly documented and configurable, 1 size will fit 85% of people, and the others can just tweak a well defined setting.
• stop all the pain with people running cron scripts to pregenerate thumbnails and gallery images which are never used!

total madness! simpler is better!

[kesselb]

Thanks for digging 👍

I remember this high dpi stuff (even for thumbnails) from many projects. Customers always complain if something looks blurry on their new MacBook Pro Retina ;)

I guess @skjnldsv and @jancborchardt are waiting for more input. If you think the high dpi logic is superfluous nowdays feel free to open a pull request with your suggested changes. I'm happy to help (e.g. with breaking tests).

Sometimes a change will improve the situation for a special case but not in general (e.g. #17028). Just to let you know 🙈

this concept of caring about the DPI for thumbnails is 6 years old!

It's probably even older because 6 years ago they move the code to this place 🤣

[nachoparker]

@dgtlmoon putting together a PR can be time consuming, so I would like to warn you. Be ready for your PR to be ignored as the gallery is clearly not a priority here.

It is awesome to contribute and I don't want to discourage you but be prepared to be ignored, you might not even get a response back.

[kesselb]

@nachoparker I'm very disappointed that you're warning someone to contribute here 😞

@dgtlmoon probably your suggestion is not accepted. It seems to be that finding the sweet spot for this preview generation topic is a hard challenge. So many people with so many different expectations.

[nachoparker]

@kesselb I love that people contribute but I also would warn them because I've seen too many times already people being enthusiastic and spending a lot of time on their PRs and being basically ignored, then them being really dissapointed.

So, not trying to prevent people from contributing, but rather warning them to the reality of how it is going to be.

[skjnldsv]

@nachoparker :(
Did we ignored some of your work?

[skjnldsv]

@rullzer mind to comment on the power of 4 thumbnails generator?

[dgtlmoon]

@skjnldsv @rullzer guys lets stay positive and move on, the whole thumbnail subsystem is junk and needs replacing, i'm working on some code here in my freetime to totally simplify it, to make it like any other PHP project that manages content

[dgtlmoon]

Who can approve the pull/commits? just one person or where do I see a list?

[kesselb]

For the preview generation I would request @skjnldsv and @rullzer as reviewer. Every member of the Nextcloud organization can approve pull requests. At least two approves and a passing ci is required to merge a pull request.

[dgtlmoon]

So... core/preview requests where the preview image ALREADY exists is taking ~200ms on my machine, I'm running a newish SSD on a Intel(R) Core(TM) i5-8250U CPU with 16Gb RAM.

What this means is that the file page, when you request the page of files, it's asking the server for 30 thumbnails...

no matter what you can only get 3 or 4 thumbnails per second to come back at 200ms each...

digging deep, i see that the entire bootstrapping process from index.php to the end of
require_once __DIR__ . '/lib/base.php'; is taking 90% of the time, the actual parsing of the thumbnails is not the problem.

so yeah... to prove my point, place this into index.php at the top, and point it to existing small image

// watch this..
if(strstr($_SERVER["REQUEST_URI"], 'core/preview')) {
	header("Content-type: image/jpeg");
	print file_get_contents("data/appdata_oc7rjprmfqh3/preview/528/150-150-crop.jpg");
	exit;
}

test request ie http:/../core/preview?fileId=532&c=....&x=325&y=325.... a single file and then also view the file list and see how insanely fast it is ie http://your-docker/apps/files/?dir=/

when you then browse the files thumbnails in the file browser app IT'S PERFECTLY FAST. (chrome reports the request completing in 8ms versus about 200ms each)

so, the bootstrap time of the entire nextcloud stack is also responsible for the slow experience, you need to focus here.

so nextcloud has some critical problems

• making unneccessary files that easily grow larger that the original data uploaded
• extremely slow bootstrap time makes any browsing and IO hurt, it's silly to have 200ms processing time on a modern machine to get a thumbnail
• wrong filesize being sent to the browser
• whacky method of generating thumbnails rounded off to the nearest 250pixel or 1000pixel size

It seems to me that the fundamental paradigm/architecture that this project is built on simply would never scale, you cant accept a 200ms RTT for a thumbnail to be something that anyone would take seriously. The codebase is horribly complicated.

To get 50 thumbnails to load within 1 second you would need a 30 core machine! what happens when you have an office of people browsing the files either via the web interface or the android/ios app? no thankyou! this slow bootstrap issue extends into all other areas of the application too.

I think the whole stack needs to be rethought, and I don't mean in a way that makes the code base even MORE complex!

Poor quality OpenSource software is a contributing factor in our decline of privacy as a civilisation - it's simply far too convenient to use an existing cloud solution that just works and is fast

@skjnldsv @rullzer @kesselb sorry but this problem is bigger than I can follow on with, as it stands I cannot recommend this PHP backend. You need to raise this whole issue at the next forum/symposium.

Cant the whole superstructure of the app (sessions, cookies, accounts, plugins) be managed by some existing PHP framework that's FAST? why is all this custom code here?

[kesselb]

You probably already know https://ownyourbits.com/2019/10/16/a-new-architecture-for-nextcloud/, #14131 and #14953. We're getting a bit off topic here. Feel free to add your findings to the other issues as additional context.

[dgtlmoon]

@kesselb thanks for that, the only thing I can say is - ColdFusion also had an event loop model and look how that went!

I dont feel that new architecture article addresses what I feel is the elephant in the room here, that there's slabs of custom code and the bootstrap time is 200ms... really strange, this is not normal. just saying "yeah! event loops!" is not really the way forwards.

Introducing more custom built code to solve a problem that has been solved by dozens of other existing frameworks that can be introduced? even Drupal 7 dropped it's custom code that it was using for over a decade and went with Symfony in version 8.

[dgtlmoon]

back to topic - I'm having a hack here https://github.com/dgtlmoon/server/tree/refactor-thumbnail-files-integration the main thing is that I'm working towards each interface (grid of thumbs, main view of image etc) issuing a type of image it wants ie &imageGroup=thumbnail and then just roll with that, instead of this bizzare overcomplicated way of inferring the DPI and weird ^n configuration (which is what is sending the wrong size image, as per this topic)

but after spending time inside the code, looking at the slabs of custom framework going on and dealing with the slowness caused by an insane bootstrap sequence, i no longer feel the motivation :( :(

[nachoparker]

@kesselb @skjnldsv I don't want to hijack this thread. I apologize if it sounded more negative than I intended to, but I am just stating a fact that a hypothetical PR will most likely not be taken into account because the gallery is not a priority for NC and that is ok. Still I think it's fair to warn before hand to a potential contributor.

Particular users get NC for free and we should be grateful, and clearly our interests do not align necessarily with the ones from NC as a company, but sometimes we fall under the illusion that because it's open source we can build this together, where this is not the case in reality. That being said, if the gallery actually worked fine it would be huge for particular users and it would take NC from "well, it's the only FOSS alternative even though it doesn't work well" to "I don't miss my proprietary services, this is awesome".

Let's leave it there, as I said I don't want to hijack the thread, and I admire people that try to fix stuff in open source to help themselves and others, just warning them.

---

PS. Not talking about myself, but about other people that have tried to help with the Gallery, some of them send me private emails in deep frustration like I can do anything about it, just because I wrote an article or two. Ofc my own PRs when related to the gallery didn't stir much movement as I would have wanted to but that doesn't bother me too much since I (and NCP users) can still benefit from my work so it didn't go to waste.

[jancborchardt]

@nachoparker Nextcloud is first and foremost a community project. Everyone is invited, welcome and in fact necessary to make this work. :)

What is a priority is decided by how many people are willing to work on improving it. We have hundreds of people in the Nextcloud organization here on Github who can review and 👍 pull requests, not only people who work for Nextcloud the company )which would create a huge bottleneck).

(Also, this issue is not about the Gallery at all, but about the file list and/or grid view.)

With that, let’s go back to the topic of the pull request. :)

❤️

[dgtlmoon]

Except that it's rounding up images to 1024pixels if the image size is greater than 256pixels... Because of the old ^n logic as above. This needs to be removed and some preset sizes put in. If you look at my branch as linked above you can see some progress.

…

On Monday, November 18, 2019, Gatak ***@***.***> wrote: One solution to the problem of file sizes is to provide a UI section in the server admin page. Something like this 1. select what thumbnail and preview dimensions should be used. Here the UI can have a predifined list or allow manual input. Explain what default sizes are used on different pages and apps, including the mobile apps. 2. select if displayed previews be loaded from the nearest below or above dimension. Give examples so the user understands the relation to point 1) 3. jpeg quality setting 0-100% I believe this would help for the time being. The issue about backend performance is another thing to be solved long term. Oh one thing. @nachoparker <https://github.com/nachoparker> optimizations for multicore rendering should be merged in nc base/server IMHO. https://ownyourbits.com/2019/06/29/understanding-and- improving-nextcloud-previews/ Thank you. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13709?email_source=notifications&email_token=AACDEONAPMCUAESI35LR3B3QUKA5LA5CNFSM4GRJ6J72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEKHNGQ#issuecomment-554989210>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACDEOPRUWDLDOE5OD5EKQ3QUKA5LANCNFSM4GRJ6J7Q> .

[nachoparker]

@Gatak

Yes, I wrote that article (I think you know that).

I played around the "using more cores" idea, just to be able to benchmark some numbers and stir some movement towards improving preview generation times, but it didn't receive much traction. I wanted to attract visibility to the issue to see if it would help.

So I played with both the generator app and the server code

nachoparker#1
#16158

nachoparker/previewgenerator#1
nextcloud/previewgenerator#166

[montvid]

So I don't understand should I use Preview Generator with default settings or what settings should I use to benefit from pregenerated thumbnails? As i understand from this discussion it is now not possible to pregenerate thumbnails?

[szaimen]

@montvid yes, pregenerating previews is recommended.

---

Those are the recommended settings, afaik:

---

This is a screenshot from:
https://ownyourbits.com/2019/06/29/understanding-and-improving-nextcloud-previews/

[szaimen]

There was a nice PR for the viewer recently nextcloud/viewer#899 but I suppose it doesn't solve this issue?

[ghost]

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.