PMXBOT Log file Viewer

Help | Karma | Search:

#pil logs for Tuesday the 5th of January, 2016

(Back to #pil overview) (Back to channel listing) (Animate logs)
[07:27:27] <jleclanche> wiredfool: hey, are you around by any chance?
[07:27:44] <jleclanche> I'm writing the dds plugin because turns out I need it
[07:27:51] <jleclanche> and I keep getting SystemError: _imaging.c:208: bad argument to internal function
[07:28:05] <jleclanche> File "/usr/lib/python3.5/site-packages/PIL/ImageFile.py", line 472, in _save
[07:28:05] <jleclanche> e.setimage(im.im, b)
[07:28:05] <jleclanche> here im.im is None
[07:29:05] <jleclanche> the api to write a custom format is so confusing. it's not even consistent between all the plugins shipped by pillow =\
[07:29:21] <jleclanche> i realize some of those go back to 1995 but there's a lot of cleanup to do really
[07:30:43] <jleclanche> (and yeah id love to help if i can)
[08:29:04] <jleclanche> this is really insane. i keep trying to take example on existing plugins and i find out that they don't work.
[09:24:41] <wiredfool> hey jleclanche
[09:24:51] <jleclanche> o/
[09:25:10] <wiredfool> Soryr for the frustration
[09:25:22] <jleclanche> np; old codebase =\
[09:25:52] <wiredfool> yeah. and I probably have the most experience with plugins, and I've really only hacked on the libtiff one.
[09:26:23] <jleclanche> so what im trying to do is in load(), decode the s3tc-compressed data into rgba
[09:26:39] <wiredfool> the two newest, _webp and Jpeg2k were contributed, and Jpeg2k is broken in threading, adn someone has sent a pr to replace the webp one.
[09:27:59] <jleclanche> i take it you saw my reply on gh?
[09:28:05] <wiredfool> yeah
[09:28:15] <wiredfool> and then I looked to the left and saw my irc window
[09:29:18] <jleclanche> so in the old PIL api what i would do is something like create a new image then do img.frombytes(...)
[09:29:25] <jleclanche> which is what the GbrImagePlugin does
[09:29:31] <jleclanche> but that plugin doesn't even work
[09:30:10] <jleclanche> right now if i set self.im to an Image object i get that bad argument to internal function error
[09:31:44] <wiredfool> you using this: https://github.com/HearthSim/python-unitypack/blob/a44c30486246df4aafc0a41adf122b5697e5cdd6/unitypack/dds.py
[09:31:50] <jleclanche> nah
[09:31:58] <jleclanche> here
[09:31:58] <jleclanche> http://sprunge.us/jCRg
[09:32:17] <jleclanche> brb a min
[09:36:35] <jleclanche> wiredfool: here's a test dds file: http://code.qt.io/cgit/qt/qtimageformats.git/tree/tests/shared/images/dds/DXT1.dds?id=a4670f609c48fed61cbacf1a0a780ea27483ce18
[09:39:35] <wiredfool> First thing, Image.frombytes returns an Image object, not a core image object. self.im is a core image object
[09:41:10] <wiredfool> Then when you overload ImageFile.load, you're clearing self.tile, and then calling super().load(), which relies on the tile for the decoding
[09:41:41] <wiredfool> the most minimal plugin that I've seen is the WebpImagePlugin
[09:42:06] <wiredfool> which decodes everything on _open, then passes a BytesIO and a raw tile to the load method
[09:42:52] <wiredfool> On the downside, that decodes everything in one shot, not doing lazy loading like the rest of Pillow. OTOH, it's dead simple and works well
[09:46:07] <jleclanche> wiredfool: perf wise im not too concerned since ideally this *would* be rewritten in c
[09:46:28] <jleclanche> wiredfool: i mean i guess the ideal method would be dxt5 / dxt1 pixel format in decode.c
[09:46:57] <jleclanche> it's just really annoying when writing a custom plugin
[09:47:14] <jleclanche> to be honest, you should be able to pass a callable instead of a decoder string
[09:47:16] <wiredfool> It's really annoying trying to understand it, even through I've done it before
[09:47:56] <wiredfool> It's way too many layers of abstraction and delegation to the c layer
[09:48:05] <wiredfool> and magic shit happening
[09:48:10] <jleclanche> yeah
[09:49:34] <jleclanche> wiredfool: how likely is a new plugin api to happen?
[09:49:41] <wiredfool> I basically have to trace things to figure it out anew each time. But to be fair, the Tiff one is as complicated as it gets. But figuring out where it converts mode to rawmode, or when it's passing anything back.
[09:52:15] <wiredfool> I don't know.
[09:56:53] <wiredfool> Dev time is ... constrained.
[09:58:54] <wiredfool> I think your problem would be solved by moving the contents of load() into _open, but setting self.fp = BytesIO(decoded_data)
[09:59:25] <wiredfool> and then just dropping the last three lines of load()
[09:59:53] <wiredfool> at that point, you can probably return the data in whatever rawmode is appropriate
[10:00:09] <wiredfool> and let the ras decoder do the work.
[10:04:54] <jleclanche> ok
[10:06:05] <jleclanche> hmm
[10:06:05] <jleclanche> File "/usr/lib/python3.5/site-packages/PIL/ImageFile.py", line 175, in load
[10:06:06] <jleclanche> self.map, self.size, d, e, o, a
[10:06:06] <jleclanche> ValueError: buffer is not large enough
[10:06:07] <jleclanche> wiredfool: ^
[10:08:10] <jleclanche> decoded_data is 10224 bytes
[10:11:52] <wiredfool> ah.
[10:18:07] <jleclanche> wiredfool: any idea?
[10:18:16] <wiredfool> looking
[10:18:28] <wiredfool> I'm thinking it's doing a saferead, and you're only getting part of the file
[10:18:57] <jleclanche> hmm
[10:19:17] <jleclanche> it's definitely calling _open only once
[10:20:25] <wiredfool> that and the test dds image was actually html
[10:20:29] <wiredfool> oops
[10:20:37] <jleclanche> :)
[10:21:48] <wiredfool> and the actual file is 1480 bytes, which is well under the saferead limit
[10:25:58] <wiredfool> Right. It's trying to mmap the file.
[10:27:32] <wiredfool> and in stupid interface decisions number 32: disabling mmap
[10:28:20] <jleclanche> huh?
[10:28:25] <wiredfool> http://pastebin.com/5PPp8FDd
[10:29:10] <wiredfool> you might ask, how can def load_seek(self,pos): pass have side effects
[10:29:18] <jleclanche> nah ive seen that in other plugins
[10:29:48] <wiredfool> ah, good.
[10:29:55] <jleclanche> great
[10:30:13] <wiredfool> I think that may be something that should be re-imagined
[10:34:51] <jleclanche> there's a lot that should be to be honest
[10:35:09] <jleclanche> decoders should just be callables, allowing for c callables
[10:35:50] <jleclanche> and there should be a predetermined API for plugins
[10:37:11] <jleclanche> wiredfool: http://sprunge.us/BFCa - CC0 so you can do whatever you like with this
[10:42:01] <wiredfool> how big are dds image files typically?
[10:45:15] <jleclanche> wiredfool: not very large
[10:45:30] <jleclanche> the biggest ones would be 2048x2048 probably but they'd be rare
[10:45:36] <jleclanche> it's used for game textures
[10:51:02] <wiredfool> so probably not too bad to just open, especially since there's no metadata in them that people are likely to want without decoding
[10:53:31] <jleclanche> well
[10:53:38] <jleclanche> like i said for any meaningful perf it needs to be done in c
[10:53:55] <wiredfool> yeah.
[10:54:11] <jleclanche> but i just needed something i could do directly from python
[10:54:21] <wiredfool> is this the one that the data's encoded in ARGB?
[10:54:28] <wiredfool> or some permutation?
[10:54:30] <jleclanche> nah that was another test
[10:54:37] <wiredfool> ok
[10:54:57] <jleclanche> take a look: https://github.com/HearthSim/python-unitypack/blob/master/unitypack/texture2d.py
[10:55:10] <jleclanche> TextureFormat enumerates what i need to support
[10:56:05] <jleclanche> i don't actually have a way to test most of those tho
[10:56:26] <wiredfool> ah
[10:56:32] <wiredfool> that's a lot of textures
[10:58:25] <wiredfool> huh. we've got unpackers for the 565 modes
[10:58:57] <wiredfool> and 5551
[10:59:22] <jleclanche> well
[10:59:26] <jleclanche> if you want a good testcase
[10:59:38] <jleclanche> making sure all of this is supported would certainly be great :p
[10:59:45] <jleclanche> and its not actually massively complicated
[11:48:43] <jleclanche> wiredfool: i filed a pr with that: https://github.com/python-pillow/Pillow/pull/1644
[11:50:14] <jleclanche> it can be improved on later I guess. I'm personally not willing to spend any more time with the pil api for my own sanity...
[11:50:21] <jleclanche> id be glad to help you design a new one though :p
[15:14:40] <travis-ci> hugovk/Pillow#955 (master - f57e295 : Andrew Murray): The build has errored.
[15:14:40] <travis-ci> Change view : https://github.com/hugovk/Pillow/compare/d392b423cd05...f57e295ce5ff
[15:14:40] <travis-ci> Build details : https://travis-ci.org/hugovk/Pillow/builds/100357762