[METATICKET] Rethinking preview_generator behavior.

[inkhey]

Current preview_generator code have many limitations:

Architecture

• Builder collision for same mimetype (https://github.com/algoo/preview-generator/issues/32), making order change the behavior of preview generator. No way to say "this builder a is better for mimetype x than builder b" (see https://github.com/algoo/preview-generator/issues/136)

• No easy matching for file type. We do rely on mimetype at builder level but we sometime need file_extension there, we rely on file_extension at manager level but we do need mimetype there. One mimetype can be linked to multiple file_extension (like jpeg image: .jpeg, .jpg, .jpe, ...) and some file_extension can be translated to multiple mimetype depending on mimetype checker (see ogg video case here: https://github.com/algoo/preview-generator/issues/142). Some fileextension also doesn't have clear mimetype like .obj file (https://en.wikipedia.org/wiki/Wavefront.obj_file) considered as mimetype 'text/plain' which mean obj can be considered as text, whereas, we will prefer generated 3d preview of them with vtk builder. We clearly need to clarify if we rely heavily on mimetype and use customized mimetype to handle non-standard type or if we should rely on another type representation. see also https://github.com/algoo/preview-generator/pull/148#discussion_r346381508

• No way to chain builder (see @JulienPalard idea there: https://github.com/algoo/preview-generator/pull/148#issuecomment-554284877), not sure it's a good idea though because it may allow creating weird preview.

• Builder are attached to mimetype, so there is no way to use one builder for text preview, one other for json one, one other for jpeg one.(see https://github.com/algoo/preview-generator/issues/136)

• check of dependencies is done at startup which is not nice for some user case (see https://github.com/algoo/preview-generator/issues/88)

• Code is currently not really modular, we should have some way to install only support for some type of file/builders : https://github.com/algoo/preview-generator/issues/37 . If we do this, it's still unclear how we split code (only python dependencies/system dependendies or more ?). We should also decide what the critar for splitting. Some builder are able to make preview for different kind of file, like imagemagick that may be able to handle text file (see https://github.com/algoo/preview-generator/issues/37) and libreoffice that can handle some image file.

• mimetype_storage is static, we should allow user to use a custom mimetype_storage (see https://github.com/algoo/preview-generator/pull/148#discussion_r348971092)

Features

• There is no png preview whereas most jpeg preview are png based.

• json preview and text preview content meaning largely differ between builder: text preview for text file is text whereas text preview for zip is list of content, json preview for ffmpeg video builder is ffmpeg metadata whereas it's exif for most other format.

• Unclear what should be the view for file like svg: page view (like cairosvg builder does) or content view (like current inkscape builder does) ?

• No easy way to set jpeg preview quality given by preview_generator and more generally, no way to "customize" preview generated.

• pdf builder relying on a pyPDF2 builder which is not well maintained, we do need a proper lib and maintained to handle pdf (qpdf ?)

• 2 Differents builder imagemagick based : Wand and IMconvert, does this already make sense ?

• No Imagemagick configuration knowledge: imagemagick builder may failed without explicit error depending on limitation and delegate configuration. (note: this seems really tricky to implement better support here)

• No Imagemagick 7 support.

• Have a Better libreoffice builder which doesn't need lock: using unoconv (https://github.com/unoconv/unoconv/issues/364) ? other solutions ?

• Pandoc builder for text format ?

So, we need to think about a refactor of preview_generator to solve all or part of theses problems.

[inkhey]

@JulienPalard Did you have any other suggestion about future preview_generator ?

[JulienPalard]

no, LGTM.

[inkhey]

Some thought about mimetype issue:

• In order to make things simpler, we probably should not rely on filepath and file_extension in most part of the application, we should use these only used to guess mimetype, then we should rely only on mimetype to get correct builder and do stuff on it. We should probably have a trick for imagemagick to force using the correct converted for the correct mimetype (imagemagick use for example "ARW" , which is similar to file_extension in this converter syntax).
• Best approach to guess mimetype seems to rely on XDG xml database, this is probably not compatible with not-linux os, so we should think about it. Pyxdg seems a good lib to do so : https://pyxdg.readthedocs.io/en/latest/mime.html, we should probably tweak a bit the https://pyxdg.readthedocs.io/en/latest/_modules/xdg/Mime.html#get_type2 method in order to handle the case where file_extension is provided outside of filepath.

[JulienPalard]

Best approach to guess mimetype seems to rely on XDG xml database

I don't trust the filename. Also relying on the filename won't work in cases we want to expose a "file-like" API in the future, like:

with open("some-file.jpg", "rb") as f:
    manager.get_jpg_preview(f)

why not relying on file content instead, à la libmagic?

[inkhey]

Best approach to guess mimetype seems to rely on XDG xml database

I don't trust the filename.

see https://specifications.freedesktop.org/shared-mime-info-spec/shared-mime-info-spec-latest.html#idm45070737710448 Not sure we should follow exactly same pattern but i do think the approach is nice to handle most case.

Keep in mind that XDG xml database means:

• really complete database and updated regularly
• handle most case of magic checking too (it's probably a bit less complete than libmagic but i do think it's good enough).
• database xml file are easy to tweak to handle custom file type (like we do for 3d file that have not real "mimetype")

As XDG cover most of the needed feature (magic, filename/extension -> mimetype), i do think it should be good enough for a first approach as the only mecanism to get mimetype. We may think about adding python magic somewhere if we see some drawbacks.

Also relying on the filename won't work in cases we want to expose a "file-like" API in the future, like:

with open("some-file.jpg", "rb") as f:
    manager.get_jpg_preview(f)

why not relying on file content instead, à la libmagic?

Nice idea, we can support this using XDG too, we should probably tweak https://pyxdg.readthedocs.io/en/latest/_modules/xdg/Mime.html#get_type2 method to be able to match this case too using https://pyxdg.readthedocs.io/en/latest/mime.html#xdg.Mime.get_type_by_data.

Another possibility is to have multiple different approach for mimetype guesser:

• XDG-like order checking spec
• content only.

[a523]

I'll say what I think:

Shouldn't we automatically detect all builders are in the project and load them all. Should we allow the user to customize which builders are enabled or disabled, like the django installed apps? Of course We can provide the option of the default enabled builders. This way there are no errors from unwanted builders, and it has the advantage that if there are multiple builders working on the same mime-type of file. The user would be free to choose which builder to enable, without requiring us to modify the library code itself.