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

Add a way to selectively disable backends #4

Closed
RKrahl opened this issue Dec 30, 2017 · 9 comments
Closed

Add a way to selectively disable backends #4

RKrahl opened this issue Dec 30, 2017 · 9 comments

Comments

@RKrahl
Copy link
Contributor

RKrahl commented Dec 30, 2017

I use vignette for a tool that works with photos. It features an overview window that uses vignette to display the thumbnails of the images. If I clear the cache to force recreation of the thumbnails, I get the following error messages for each image when I open the overview window:

Syntax Warning: May not be a PDF file (continuing anyway)
Syntax Error: Couldn't find trailer dictionary
Syntax Error: Couldn't find trailer dictionary
Syntax Error: Couldn't read xref table
Error opening PDF file.

It's certainly not surprising that the PDF backend fails on a JPG file. The creation of the thumbnails succeeds nevertheless. But the error messages are still annoying. So I disable the backends that are not useful for my application with:

for backcls in (vignette.OooCliBackend, 
                vignette.PopplerCliBackend, 
                vignette.FFMpegCliBackend):
    for i in reversed(range(len(vignette.THUMBNAILER_BACKENDS))):
        if isinstance(vignette.THUMBNAILER_BACKENDS[i], backcls):
            del vignette.THUMBNAILER_BACKENDS[i]

This also speeds up creating the thumbnails by a factor of more then two.

While this works, fiddling that deep in the guts of a third party library is certainly not good practice. So I'd like to get a better solution to select the backends that should be tried.

@hydrargyrum
Copy link
Owner

Indeed, backends are all tried without distinction until one accepts the input.

ThumbnailBackends could have a is_supported method to check if the thumbnailing operation is likely to succeed. This method could use libmagic if it's installed to determine if the file format is supported, and fallback on checking file extension else (I don't like that too much but I can't come up with a better idea).
is_supported would be called internally by vignette, so an app using vignette doesn't have to be modified.
The drawback is that if libmagic isn't installed, an image file without an extension will not be thumbnailed.
(Now that I check, it seems there's a mess of multiple libmagic backends, but that's another question)

What do you think?

@hydrargyrum
Copy link
Owner

Without using libmagic yet but only plain old mimetypes module, here's some working code: https://github.com/hydrargyrum/vignette/commits/mime-accept (mime-accept branch)

@RKrahl
Copy link
Contributor Author

RKrahl commented Dec 31, 2017

Verifying if the type of the file in question is suitable for a particular backend before starting it, is certainly useful on its own. So I welcome your idea.

But it is not the same what I was suggesting in this issue: for general purpose applications such as a file manager, it certainly makes sense trying to cover as much file types as possible. But for specialized applications such as mine that will ever only deal with one single type of files, such as images in my case, it makes sense to disable all backends not dealing with images from the beginning. So what I was requesting is a mechanism that would allow the calling application to tell vignette to only use backends dealing with a particular set of file types.

The requirements for such a mechanism would be:

  • the calling application should not need to know what backends are present in vignette,
  • the mechanism should be based on file types, mimetypes is certainly a good approach to classify these types.

A sketch of such a mechanism could be:

  1. Define constants FTYPE_IMG, FTYPE_PDF, … for all file types that are supported by any backend. These constants must be hashable, but are otherwise opaque.
  2. Each backend has the set of supported file type constants defined as a class attribute.
  3. A global module variable contains the list of currently active backends. It defaults to all (available) backends. (A list rather then a set because we want a predictable order of application.)
  4. A function select_backends(ftypes) may be called by the application. It takes a set of file type constants as argument and sets the global list of active backends to the backends whose set of supported file types has a non-empty intersection with the file types given as argument.
  5. When creating a thumbnail, try only the backends in the list of active backends.

@hydrargyrum
Copy link
Owner

Ok I see.
The constants should be type categories (e.g. image, video, document, archive, etc.) rather than individual types like pdf? If a specific new file type is implemented, it'll fall in a category and apps using this category won't have to be updated to declare the new file type.

hydrargyrum added a commit that referenced this issue Jan 9, 2018
An app using vignette could want to only keep a category of handled
files. See #4
@hydrargyrum
Copy link
Owner

hydrargyrum commented Jan 9, 2018

@RKrahl what do you think of https://github.com/hydrargyrum/vignette/commits/selective ? Would it suit your needs?

@RKrahl
Copy link
Contributor Author

RKrahl commented Jan 9, 2018

Sorry for not responding to your last comment. I'm somewhat busy atm. I'll try to find some time to have a look.

@hydrargyrum
Copy link
Owner

Ok, no worries.

hydrargyrum added a commit that referenced this issue Aug 17, 2019
An app using vignette could want to only keep a category of handled
files. See #4
@hydrargyrum
Copy link
Owner

@RKrahl I've just merged 2 big changes, the one you asked for: being able to select specific file types, and another for not starting every plugin for every file but only the most appropriate (by using MIME).
Feel free to give feedback

@RKrahl
Copy link
Contributor Author

RKrahl commented Sep 1, 2019

Many thanks! I tried it and it works fine for me.

@RKrahl RKrahl closed this as completed Sep 5, 2023
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

No branches or pull requests

2 participants