Narrat / Pim

PyGObject based image viewer with vim-like keybindings
MIT License
28 stars 9 forks source link

Bitmaps #8

Closed Narrat closed 8 years ago

Narrat commented 8 years ago

Stumpled upon bitmaps (.bmp) and was kinda suprised pim didn't open those.

Explicit calling those behaved like giving a directory

karlch commented 8 years ago

First of all thanks for your great work on Pim, I have highly enjoyed working with the codebase.

I did a quick debug on this. Looks like not all .bmp mimetypes are included in GdkPixbuf.Pixbuf.get_formats(). In my specific case, I tested with a few bitmaps exported from gimp, the "image/x-ms-bmp" type was missing. Simply appending that type to the supported ones solved the issue.

The problem is, I have no idea how many more types are missing and how one could find out what they are.

Narrat commented 8 years ago

Such kind words :D Although James Campos (aeosynth) should be praised mostly. As I just worked on his ToDo entries later. And it was nice to see, someone taking this as base and enhancing it with various features.

And thanks for debugging this. Currently I don't have the time to check on this. And if it's related to GdkPixbuf dunno what's the best thing to do.

Maybe some kind of regex, if the first check returned as unsupported, check if there are similar ones --> will be added and failure will be sorted out later as it could be covered by the routine with broken images. Whatever hell pit such a handling will open :D Just some quick thought on this

aeosynth commented 8 years ago

can you share some failing bitmaps? would checking extension (.bmp) instead of mime type fix this?

Narrat commented 8 years ago

https://paste.archlinux.de/m-gObM/

Well. Checking the extension would open the problem with non image files with an .bmp ending (for whatever reason those could exist)

Edit: Or without any extension

aeosynth commented 8 years ago

those have the type image/x-ms-bmp; GdkPixbuf supports image/x-MS-bmp. a quick fix would be to lowercase the types (x-MS-bmp is the only type using caps). using extensions is easier tho; users can rename their files if they have issues

karlch commented 8 years ago

Just for sharing information: I had someone open an issue for opening images without any extension https://github.com/karlch/vimiv/issues/17

In general I agree with him, just filtering images by mimetype is not very sophisticated. Using the imghdr module might be a better alternative. I plan on testing that sometime this or next month, still pretty busy at the moment.

If it works and you are interested, I could probably implement a PR for Pim afterwards as well.

aeosynth commented 8 years ago

you'd have to convince me by showing images which are only recognizable with that module

karlch commented 8 years ago

Got to it earlier than expected.

Say you have a correct imagefile called "myimage" and a textfile called "text.png". The extension version wouldn't work whereas imghdr filters correctly.

As the opened issue shows the first case happens, the second one is rather esoteric but nice to be protected against. Sure, one could rename the files manually, but this shouldn't be necessary.

I have yet to find a file that imghdr does not handle correctly.

aeosynth commented 8 years ago

The extension version wouldn't work whereas imghdr filters correctly.

what about the mimetype version?

i'd like to see real images which are detected differently

karlch commented 8 years ago

Just rename any images of your choice accordingly :smiley:

I don't have a favourite pastebin yet...

aeosynth commented 8 years ago

oh, i thought mimetypes actually read from disk

aeosynth commented 8 years ago

http://lazka.github.io/pgi-docs/GdkPixbuf-2.0/classes/Pixbuf.html#GdkPixbuf.Pixbuf.get_file_info

Parses an image file far enough to determine its format and size.

i believe this can be used instead of imghdr

karlch commented 8 years ago

Nice catch! First tests with it seem to go perfectly well.