Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preview formats no longer sufficient for photo app 2.0.0 #1492

Open
markuman opened this issue Nov 23, 2022 · 7 comments
Open

Preview formats no longer sufficient for photo app 2.0.0 #1492

markuman opened this issue Nov 23, 2022 · 7 comments
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working performances Performances issues and optimisations

Comments

@markuman
Copy link

Describe the bug

nextcloud 25.0.1

/var/www/nextcloud$ sudo -u www-data php occ app:list|grep -E 'photo|preview'
  - photos: 2.0.0
  - previewgenerator: 5.1.1

previewgenerator settings (works with nextcloud 24 and photos 1.6.0)

sudo -u www-data php occ config:app:set --value="32 64 1024"  previewgenerator squareSizes
sudo -u www-data php occ config:app:set --value="64 128 1024" previewgenerator widthSizes
sudo -u www-data php occ config:app:set --value="64 256 1024" previewgenerator heightSizes

previewgenerator 5.1.1. produces in nextcloud 25.0.1 with photo app 2.0.0 such files

data/appdata_ocn8kt6uce4g/preview/
├── 0
│   ├── 0
│   │   ├── 0
│   │   │   ├── 6
│   │   │   │   └── a
│   │   │   │       └── a
│   │   │   │           └── b
│   │   │   │               └── 18829
│   │   │   │                   ├── 1024-768-max.jpg
│   │   │   │                   ├── 256-192.jpg
│   │   │   │                   ├── 341-256.jpg
│   │   │   │                   ├── 64-48.jpg
│   │   │   │                   ├── 64-64-crop.jpg
│   │   │   │                   ├── 768-768-crop.jpg
│   │   │   │                   └── 85-64.jpg

However, when the photo app is opened, previews are still generatoed on-the-fly which produces a very high CPU load and the entire photoapp/nextcloud becomes unuseable.
The generated previews are still sufficient for the memories app https://github.com/pulsejet/memories

When I remove all previews /var/www/nextcloud$ sudo rm -rf data/appdata_ocn8kt6uce4g/preview/,reset everything using /var/www/nextcloud$ sudo -u www-data php occ files:scan-app-data and reopen the photo app in a private tab of a web browser, the following files are produces due the on-the-fly process.

├── 1
│   └── 7
│       └── b
│           └── 6
│               └── 5
│                   └── a
│                       └── f
│                           └── 6326
│                               ├── 1024-1365-max.jpg
│                               ├── 48-64.jpg
│                               └── 768-1024.jpg

Desktop (please complete the following information):

  • OS: Ubuntu 22.04
  • Browser Firefox, Chromium
  • Version latest

Browser log

Open your console, reload your page and/or do the action leading to this issue and copy/paste the log in this thread.
How to access your browser console (Click to expand)

Chrome

  • Press either CTRL + SHIFT + J to open the “console” tab of the Developer Tools.
  • Alternative method:
    1. Press either CTRL + SHIFT + I or F12 to open the Developer Tools.
    2. Click the “console” tab.

Safari

  • Press CMD + ALT + I to open the Web Inspector.
  • See Chrome’s step 2. (Chrome and Safari have pretty much identical dev tools.)

IE9

  1. Press F12 to open the developer tools.
  2. Click the “console” tab.

Firefox

  • Press CTRL + SHIFT + K to open the Web console (COMMAND + SHIFT + K on Macs).
  • or, if Firebug is installed (recommended):
    1. Press F12 to open Firebug.
    2. Click on the “console” tab.

Opera

  1. Press CTRL + SHIFT + I to open Dragonfly.
  2. Click on the “console” tab.

Additional context
Add any other context about the problem here.

@markuman markuman added 0. Needs triage Pending approval or rejection. This issue is pending approval. bug Something isn't working labels Nov 23, 2022
@LaXiS96
Copy link

LaXiS96 commented Mar 23, 2023

Hello there,
I investigated the preview generation behavior of Nextcloud 25 and this is what I found:

  • generic previews (dashboard, recommended files, ...) ask for 250x250 cropped previews
  • the Files app asks for 256x256 cropped previews, but the size scales with the browser zoom (at 130% -> 334x334)
  • the Photos 2.0.1 app asks for 64x64 and 512x512 previews with original aspect ratio
  • the Maps 1.0.0 app asks for 341x256 previews with original aspect ratio

The built-in preview generator (https://github.com/nextcloud/server/blob/v25.0.4/lib/private/Preview/Generator.php) was updated to generate previews at intervals of powers of 4 instead of powers of 2.

Additionally, if you have Imaginary set up, any request where width or height are <=256 will result in a 256px max preview being generated (for both cropped and original aspect ratio). This means that previews smaller than 256 will not be used, even if they were already generated by previewgenerator.
For example, I have Imaginary set up and my Photos app, despite asking for 64x64 previews, only receives 256 previews.

The size calculation logic is all in here: https://github.com/nextcloud/server/blob/v25.0.4/lib/private/Preview/Generator.php#L415
I tested the code with the sizes I listed above and this is the result for an image with resolution of 2000x1333:

  • 64x64 original -> 64x43, but 256 with Imaginary
  • 250x250 cropped -> 256x256 crop
  • 256x256 cropped -> 256x256 crop
  • 334x334 cropped -> 1024x1024 crop
  • 341x256 original -> 384x256, but 256 with Imaginary
  • 512x512 original -> 1024x682

previewgenerator is missing a few things:

  • it does not generate the max preview, while the built-in generator pretty always does
  • it duplicates non-cropped previews: the built-in generator only creates a single preview by limiting the biggest dimension, while previewgenerator always builds two by limiting one dimension at a time (widthSizes and heightSizes)
  • it does not consider that for <=256px non-cropped previews with Imaginary, the built-in generator generates a 256-256 named file, even if the aspect ratio is not square and either one of the dimensions is not 256... this was a bad move on Nextcloud's side

I'm not sure what would be the best way to ensure previewgenerator is creating the correct sizes without overdoing...

@szaimen
Copy link
Contributor

szaimen commented Mar 24, 2023

Cc @st3iny

@LaXiS96
Copy link

LaXiS96 commented Mar 24, 2023

I was just looking into the built-in preview generation source code and noticed something that looked promising, namely preview_concurrency_all and preview_concurrency_new system settings, which were introduced with nextcloud/server#18210 and released in 26.0.0.

Looks like with Nextcloud 26 my server won't go down because of on-the-fly preview generation :)
But it would still be nice for previewgenerator to pre-generate the correct ones, thank you @st3iny

@LaXiS96
Copy link

LaXiS96 commented Mar 24, 2023

Quick update: with Nextcloud 26 I can finally use the Photos app! 🎉🎉🎉
The fix was long overdue (first reported in 2019: nextcloud/server#15075) but the new concurrency limit works wonders: previews are a bit slow to be generated (for new images) but the server load is manageable and does not bog down the system!

@st3iny
Copy link
Member

st3iny commented Mar 27, 2023

@LaXiS96 Thank your for your research on preview sizes. I'll try to incorporate the changes when time permits.

TBH, I think that previewgenerator is becoming obsolete since the preview parallel limit was implemented. The app originally tried to solve the problem that is now solved: Extreme server load due to massively parallel preview generation.

@szaimen
Copy link
Contributor

szaimen commented Mar 27, 2023

TBH, I think that previewgenerator is becoming obsolete since the preview parallel limit was implemented. The app originally tried to solve the problem that is now solved: Extreme server load due to massively parallel preview generation.

This is my POV as well :)

@markuman
Copy link
Author

TBH, I think that previewgenerator is becoming obsolete since the preview parallel limit was implemented. The app originally tried to solve the problem that is now solved: Extreme server load due to massively parallel preview generation.

This is my POV as well :)

I think it depends. If you've got thousands, maybe hundred thousands images and videos, it will took ages until you'll see any previews if you fast scrolldown. Yes, the system won't die anymore.
But I think the power users still need a previewgenerator.

@joshtrichards joshtrichards added performances Performances issues and optimisations 1. to develop Accepted and waiting to be taken care of and removed 0. Needs triage Pending approval or rejection. This issue is pending approval. labels Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug Something isn't working performances Performances issues and optimisations
Projects
None yet
Development

No branches or pull requests

5 participants