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

Moved creation of thumbnail urls from MediaRepresentation to ThumbnailManager. #2140

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Daniel-KM
Copy link
Contributor

To make media more generic, the creation of the thumbnail urls may be moved into the thumbnail manager. It will make creation of thumbnails more flexible, in particular for media iiif, where the image server can create thumbnail itself, but it will be for a next pr if this one is passed.

@Daniel-KM
Copy link
Contributor Author

And it will fix https://forum.omeka.org/t/thumbnail-creation-with-iiif-image-media/18584 (thumbnails don't need to be stored locally by omeka, but created dynamically (or cached) by the image server.

@Daniel-KM Daniel-KM force-pushed the feature/media_thumbnail branch 2 times, most recently from 28e6f08 to e0d309c Compare January 18, 2024 19:10
@olivierghuzel
Copy link

This development would be very welcome!

Large loadings of IIIF media are extremely slowed down by the creation of thumbnails. But these are often superfluous.

The high definition version of the image is downloaded and then processed to create the thumbnails. In many cases, the IIIF server can provide the same service more efficiently, and with a saving in disk space.

I hope it can be done!

@zerocrates
Copy link
Member

zerocrates commented Jan 19, 2024

Can you give a preview or discussion of what the feature this is intended to enable will look like, code-wise?

Hoping to get a sense of why it's useful/necessary to move the URL code here.

@Daniel-KM
Copy link
Contributor Author

The first idea is purely technical: to move everything related to thumbnail in the same place. Since all media are no more files like Omeka Classic, the thumbnails should be managed outside of the media representation.

This transparent refactorisation will make the creation of the url of the thumbnails overridable by modules. So the module may add a check for the media ingester iiif image in the method thumbnailUrl(), then get the url of the thumbnail provided by the image server to avoid to create static thumbnails, that are useless in big bases. This code can be implemented in the core anyway, since the format of the urls are standard in IIIF and iiif data of the image are available. And this change will open more possibilities too, for example to use more fallback thumbnails or to use webp instead of jpeg.

The code can be improved via delegators, or via an event, but the move avoids any side effects so it can be integrated for v4.1.

@zerocrates
Copy link
Member

Just so I'm clear, from a purely functional standpoint, the benefit to moving the method here is that it's then in a service and you can more easily override a service than the representation?

@Daniel-KM
Copy link
Contributor Author

Yes, representations are the core of omeka so it should not be overridden, but a thumbnail manager is something less essential.

Ideally, nothing should be overridden, but for now, it is not possible to modify the way the url of thumbnails are created without modifying the core. If you prefer something else, tell me.

@Daniel-KM
Copy link
Contributor Author

So in thumbnailUrl() (or equivalent in MediaRepresentation):

if ($media->ingester() === 'iiif') {
    // To be done.
    return $this->getServiceLocator()->get('IiifImage')->thumbnailUrl($media, $type);
}
// else current code...

And the iiif ingester can maybe be set "has no thumbnail".

@zerocrates
Copy link
Member

Right, OK, I see the general idea.

Let me think it over a bit and get back to you... thinking about the interaction of this, hasThumbnails, needing to skip the thumbnail generation in the first place, etc.

Some hesitancy for having the thumbnail manager have knowledge of the representation... thinking also if we should just loop in the ingester and/or renderer here.

@Daniel-KM
Copy link
Contributor Author

Yes, and the idea is the same for a media youtube, where you can use a thumbnail from youtube instead to install ffmpeg on the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants