Issue 4663693082099712: Issue 1897 - Compress auto-generated images with pngout

Issue 4663693082099712: Issue 1897 - Compress auto-generated images with pngout (Closed)

Created:
Jan. 28, 2015, 9:40 p.m. by Sebastian Noack

Modified:
Feb. 10, 2015, 8:49 p.m.

Reviewers:
kzar, Wladimir Palant

Visibility:
Public.

Description

Issue 1897 - Compress auto-generated images with pngout

Patch Set 2 : Fixed grammar in comment #

Total comments: 3

Created: Feb. 10, 2015, 9:57 a.m.

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+89 lines, -5 lines)			Patch
	A	imageCompression.py	View	1	1 chunk	+82 lines, -0 lines	3 comments	Download
	M	imageConversion.py	View		3 chunks	+7 lines, -5 lines	0 comments	Download

Messages

Total messages: 6

Expand All Messages | Collapse All Messages

kzar

Otherwise LGTM http://codereview.adblockplus.org/4663693082099712/diff/5629499534213120/imageCompression.py File imageCompression.py (right): http://codereview.adblockplus.org/4663693082099712/diff/5629499534213120/imageCompression.py#newcode62 imageCompression.py:62: file.name = filename # Set the 'name' ...

Feb. 10, 2015, 9:29 a.m. (2015-02-10 09:29:54 UTC) #2

Sebastian Noack

http://codereview.adblockplus.org/4663693082099712/diff/5629499534213120/imageCompression.py File imageCompression.py (right): http://codereview.adblockplus.org/4663693082099712/diff/5629499534213120/imageCompression.py#newcode62 imageCompression.py:62: file.name = filename # Set the 'name' attribute, that ...

Feb. 10, 2015, 9:57 a.m. (2015-02-10 09:57:38 UTC) #3

Wladimir Palant

http://codereview.adblockplus.org/4663693082099712/diff/5178081291534336/imageCompression.py File imageCompression.py (right): http://codereview.adblockplus.org/4663693082099712/diff/5178081291534336/imageCompression.py#newcode41 imageCompression.py:41: self._thread.start() Why not use subprocess.communicate()? The files aren't large ...

Feb. 10, 2015, 6:47 p.m. (2015-02-10 18:47:59 UTC) #4

Sebastian Noack

http://codereview.adblockplus.org/4663693082099712/diff/5178081291534336/imageCompression.py File imageCompression.py (right): http://codereview.adblockplus.org/4663693082099712/diff/5178081291534336/imageCompression.py#newcode41 imageCompression.py:41: self._thread.start() On 2015/02/10 18:47:59, Wladimir Palant wrote: > Why ...

Feb. 10, 2015, 7:23 p.m. (2015-02-10 19:23:56 UTC) #5

Wladimir Palant

Feb. 10, 2015, 8:49 p.m. (2015-02-10 20:49:18 UTC) #6

http://codereview.adblockplus.org/4663693082099712/diff/5178081291534336/imag...
File imageCompression.py (right):

http://codereview.adblockplus.org/4663693082099712/diff/5178081291534336/imag...
imageCompression.py:41: self._thread.start()
On 2015/02/10 19:23:56, Sebastian Noack wrote:
> So you mean I should save the image into a StringIO, get its value to pass it
to
> communicate()? Sounds kinda like a hack, and not really simpler.

Seems a lot simpler than running a thread IMHO, especially if image_to_file
returns a string rather than a file object (the caller will call read() on it
anyway). Sure, the function needs to be renamed then. But converting the image
to string will always be the first step - whether with compression or without.

Expand All Messages | Collapse All Messages