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

Thumbnails management enhancement #10808

Closed
mxbossard opened this issue Sep 1, 2014 · 17 comments
Closed

Thumbnails management enhancement #10808

mxbossard opened this issue Sep 1, 2014 · 17 comments

Comments

@mxbossard
Copy link

mxbossard commented Sep 1, 2014

Hello,
I'm new to owncloud, but i am really interested in it. I began to store a lot of pictures, and I noticed several problems with thumbnails :

1- The thumbnails seems not be attached to files but to file paths, witch are relatives. If I rename a folder all thumbnails in it have to be rebuild or at least redownload.
2 - When I share a folder, the same problem appear, I think the linked path is used for downloading thumbnails instead of an uid.

I don't know yet how the file uid model is designed in owncloud, but I think file related features should be attached to file uid, and not to file path which is relative. It could save time consuming tasks to be performed several times instead of one.

Moreover, thumbnails are so tiny, the number of concurrent http requests needed to load 1kB of thumbnailsfor each row is really browser perf consuming. You should consider to return some HTTP/403 when we know the file was not modified instead of redownloading the thumbnails every time.

@deMattin
Copy link

deMattin commented Sep 2, 2014

And there would be other advantages (possible) with using uids instead of the account based path/file structure:

Now every account creates own thumbnails (and gallery previews) for shared files owned by another user.
With uids this won't be necessary any more, if thumbnails are created account independent in a separate folder outside the account folder!
Further more this will increase performance and reduces server load and storage space use, because any thumbnail has only to be created once for shared files.

I think using the file uids for thumbnails (and gallery app) would simplify code, make it straight forward and even avoid possible errors in code (like the actual renaming bug #10747 and many others related to shared folders and previews/thumbnails pathes).

I also have wondered, why file uid isn't used for thumbnails and gallery app.

@PVince81
Copy link
Contributor

PVince81 commented Sep 2, 2014

From what I saw, some thumbnails are stored based on the file id, but from what I heard in other reports it seems that other thumbnails are based on the file path.
It might make sense, as suggested above, to make them all based on the file id.

There is also a difference between the file thumbnails and the gallery app's cached pictures.
I assume we're talking only about the file thumbnails here. (for now)

@georgehrke what do you think ?

@georgehrke
Copy link
Contributor

From what I saw, some thumbnails are stored based on the file id, but from what I heard in other reports it seems that other thumbnails are based on the file path.
It might make sense, as suggested above, to make them all based on the file id.

The preview system (everything stored in userfolder/thumbnails) uses the fileId. Not sure if gallery has a dedicated thumbnail cache. cc @icewind1991

There is also a difference between the file thumbnails and the gallery app's cached pictures.
I assume we're talking only about the file thumbnails here. (for now)

AFAIK the gallery shipped with ownCloud 6 uses it's own preview management. Gallery's master is already migrated to the preview system. Stable7 as well I guess. cc @icewind1991

@georgehrke
Copy link
Contributor

1- The thumbnails seems not be attached to files but to file paths, witch are relatives. If I rename a folder all thumbnails in it have to be rebuild or at least redownload.
2 - When I share a folder, the same problem appear, I think the linked path is used for downloading thumbnails instead of an uid.

The url (preview.png ...) uses the path because we don't want to expose the fileId, but internally every caching is based on the fileId

@georgehrke
Copy link
Contributor

Now every account creates own thumbnails (and gallery previews) for shared files owned by another user.

That's already the case

@mxbossard
Copy link
Author

@georgehrke
But the fact the URL is based on the path prevent the browser cache to be used.

@georgehrke
What is the point for each user to have their own version of thumbnails ?

I have another idea about thumbnails. It may be interesting for perf to build a sprite for about 2 pages of listing thumbnails. Sprite would be a concatenation of about 20 thumbnails. So only one http request would be send to download nearly 30 kB of images for 20 thumbnails. CSS would be used to display the goo thumbnail in the sprite.
The problem here with this approached is to keep in sync the sprite with the folder content. It could be possible to make grow a sprite, by appending more and more thumbnails at the end of the sprite in a certain limit. If we add a file in the first folder page, we add a 21st pic in the first sprite for example. If we remove a file in the folder we can keep the sprite unchanged and not display the thumbnails. After too much entropy we can rebuild the sprite.

@georgehrke
Copy link
Contributor

But the fact the URL is based on the path prevent the browser cache to be used.

The response of preview.png is cached by the browser for 24 hours

@mxbossard
Copy link
Author

The response of preview.png is cached by the browser for 24 hours

But if the folder is renamed the path change so the URL and the browser cache index.

@georgehrke
Copy link
Contributor

But if the folder is renamed the path change so the URL and the browser cache index.

Yes, then it requests preview.png again. But the preview is also cached on the server. We don't generate a completely new one

@mxbossard
Copy link
Author

Yes, the it requests preview.png again. But the preview is also cached on the server. We don't generate a completely new one

Yup, but we download all the thumbnails again(I think it's the case for all the files not only thumnails).

Why not exposing the fileId ? Security should not be based on hiding the fileId :). At least, if we cannot use the fileId, we could generate a per user identifier wich is different from the fileId and from the path but stay constant over time.

@oparoz
Copy link
Contributor

oparoz commented Jan 22, 2015

@mxbossard - How many times a day do you rename or move your folders? Worst case scenario, the browser reloads hundreds of 36x26 pics. Shouldn't be that much of an issue, no? It will have to do it every day anyway due to the max 24h caching policy

@Spacefish
Copy link

I wrote a cron to pregenerate the thumbnails on my instance. On that occassion i was also confused on why the thumbnails are stored per user and based on the fileId. After i truncate oc_filecache (which i do from time to time to speed up owncloud again) all thumbs had to be regenerated. So i changed the thumbnail identifier to absolut path on file system and generated the thumbs only once so i have a thumbnails folder in my root owncloud instance wide data path. because paths are to complicated / long i just md5 hash the paths and use parts of the md5 hash to do a two layer deep folder structure. So far so good, it works nice :) the cron generates 100 thumbs per run so it doesn´t block the box for a long time.

However this is not an optimal solution in my eyes, as the "thumbfolderhash" changes as the path changes.. A optimal solution IMHO would be to use the hash of the file itself the thumbnail is created from. same hash -> same thumbnail so this even does thumbnail deduplication 💃

What i don´t understand why the thumbails are generated per User folder in the first place instead of instance wide.. Is there any logical reason behind that? For my installation every feature seems to work even with the changes.. (Sharing Folder / Links and so on).

@PVince81
Copy link
Contributor

Arghh, never truncate the oc_filecache table. You will lose all shares, favorite info, etc.

@ooZberg
Copy link

ooZberg commented Jan 10, 2016

I have also noticed that all users need to generate their own thumbnails (and gallery images), which is very annoying if you have 100Gb+ photos that are shared with multiple users.

I agree that it would be nice to have a common thumbnails cache folder indexed by e.g. file hash, but that might also take some time to calculate for each file.

Wouldn't an easier fix be to:

  • Identify if the current file is a shared file or not, and if it is shared, use the thumbnail cache from the owner's data folder.
  • If the owner does not have a thumbnail generated, generate the new thumbnail in the owner's folder.

This way the thumbnails will only be generated once. Would this make sense to implement?

@oparoz
Copy link
Contributor

oparoz commented Jan 11, 2016

It's not going to happen any time soon because there are too many special cases to consider: #17186

@PVince81
Copy link
Contributor

closing in favor of #28939

@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests