Register Guidelines E-Books Search Today's Posts Mark Forums Read

Go Back   MobileRead Forums > E-Book Software > Sigil > Plugins

Notices

Reply
 
Thread Tools Search this Thread
Old 02-03-2016, 09:55 AM   #16
Toxaris
Wizard
Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.
 
Toxaris's Avatar
 
Posts: 4,521
Karma: 121692313
Join Date: Oct 2009
Location: Heemskerk, NL
Device: PRS-300, PRS-T1
Quote:
Originally Posted by Doitsu View Post
Can you please check the version number of jpegtran? Toxaris's plugin uses version 9, which uses a different syntax than the previous version which accepts only one file name parameter.

The old version has the following syntax:

Code:
usage: jpegtran [switches] [inputfile]
however, the new version uses the following syntax:

Code:
usage: jpegtran [switches] [inputfile] [outputfile]
If you use the new syntax with the old version, it enters an endless loop, which causes the plugin to hang.

AFAIK, libjpeg-turbo is a fork of jpegtran that uses the old syntax. I.e. you can't use it to optimize .jpeg files under Linux.
Actually, this is not the syntax I use. I specify the switch outfile and that should work anyway. However, I will probably use the alternative that DiapDialer mentioned to make it more flexible.
Toxaris is offline   Reply With Quote
Old 02-03-2016, 10:12 AM   #17
eschwartz
Ex-Helpdesk Junkie
eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.
 
eschwartz's Avatar
 
Posts: 19,287
Karma: 83106505
Join Date: Nov 2012
Location: The Beaten Path, USA, Roundworld, This Side of Infinity
Device: Kindle Touch fw5.3.7 (Wifi only)
Quote:
Originally Posted by Doitsu View Post
Can you please check the version number of jpegtran? Toxaris's plugin uses version 9, which uses a different syntax than the previous version which accepts only one file name parameter.
Looks to me like it uses the old syntax, actually.
Code:
    elif (filename.lower().endswith('.jpg') or filename.lower().endswith('.jpeg'))    :
        # get correct arguments
        if iswindows:
            exe_path = os.path.join(SCRIPT_DIR,'win', 'jpegtran.exe')
        elif isosx:
            exe_path = os.path.join(SCRIPT_DIR,'osx','jpegtran')
        else:
            exe_path = 'jpegtran'
                
        args = [exe_path]
        args.append("-optimize")
        args.append("-progressive")
        args.append("-copy")
        args.append("none")
        args.append("-outfile")
        args.append(filename)
        args.append(filename)
The outfile is passed via -outfile, not as the last argument... exactly as both libjpeg-turbo and mozjpeg expect.
In fact, you can use libjpeg's version 9 executable with either syntax. Which is I guess why Toxaris used the old syntax.

Quote:
The old version has the following syntax:

Code:
usage: jpegtran [switches] [inputfile]
however, the new version uses the following syntax:

Code:
usage: jpegtran [switches] [inputfile] [outputfile]
If you use the new syntax with the old version, it enters an endless loop, which causes the plugin to hang.

AFAIK, libjpeg-turbo is a fork of jpegtran that uses the old syntax. I.e. you can't use it to optimize .jpeg files under Linux.
What -- because it uses the old syntax, this file optimizer doesn't work on linux? Or the plugin cannot use it?

Because I am pretty sure both are untrue.

Also, on my machine, using the libjpeg version 9 syntax with libjpeg-turbo (or mozjpeg) does exactly what I expect -- it immediately returns a failure:
Code:
jpegtran: only one input file
usage...
Quote:
Originally Posted by DiapDealer View Post
That way the user can just modify the json prefs file to 1) point to a newer version of jpegtran (which I'm betting calibre's is); or 2) use wine to run the included Windows version.
calibre uses jpegtran from the latest version of mozjpeg, rather than using libjpeg-turbo (the latest version of which is on roger64's machine).
mozjpeg and libjpeg-turbo are both equally NOT libjpeg.




....




I suppose the real question is, why are you using subprocess.call(list(args), shell=True)

subprocess.call(args) works fine without any other changes.
(Don't use a shell. Minor nit: don't convert a list into a list)
eschwartz is offline   Reply With Quote
Old 02-03-2016, 10:25 AM   #18
Toxaris
Wizard
Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.
 
Toxaris's Avatar
 
Posts: 4,521
Karma: 121692313
Join Date: Oct 2009
Location: Heemskerk, NL
Device: PRS-300, PRS-T1
Updated version which includes a preference file to chose another version of jpegtran and the usage of wine.
Toxaris is offline   Reply With Quote
Old 02-03-2016, 10:50 AM   #19
Doitsu
Wizard
Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.
 
Doitsu's Avatar
 
Posts: 4,580
Karma: 14563843
Join Date: Dec 2010
Device: Kindle PW2
Quote:
Originally Posted by eschwartz View Post
calibre uses jpegtran from the latest version of mozjpeg, rather than using libjpeg-turbo (the latest version of which is on roger64's machine).
mozjpeg and libjpeg-turbo are both equally NOT libjpeg.
It appears that I was mistaken. When the jpeg optimization didn't work on my machine I checked the jpegtran version number and found out that it was jpegtran 8d, but the Windows version bundled with the plugin is 9a.

I then executed the same command line that the plugin uses:

Code:
jpegtran -optimize -progressive -copy none -outfile /tmp/tmp2RD8h1/image.JPG /tmp/tmp2RD8h1/image.JPG
and noticed that the execution entered an endless loop. Since the plugin didn't work on roger64's Arch Linux machine and he said that he's installed libjpeg-turbo, I erroneusly assumed that libjpeg-turbo might have the same problem as my jpegtran 8d version.

Did you install the ImageOptimizer plugin on your machine and does it work with libjpeg-turbo?
Doitsu is offline   Reply With Quote
Old 02-03-2016, 10:58 AM   #20
eschwartz
Ex-Helpdesk Junkie
eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.
 
eschwartz's Avatar
 
Posts: 19,287
Karma: 83106505
Join Date: Nov 2012
Location: The Beaten Path, USA, Roundworld, This Side of Infinity
Device: Kindle Touch fw5.3.7 (Wifi only)
Again, it works fine once I fixed the plugin.

The problem is that subprocess.Popen expects either:
  1. a list, and shell=False (shell defaults to false)
  2. a string, and shell=True

Passing a list to the shell means it tries to execute "jpegtran" and pass the additional arguments as arguments to the shell itself, not jpegtran.

And yes, when you run
Code:
jpegtran
it hangs.

Why on earth does it specifically need a shell? The plugin isn't using shell builtins, pipes, variable expansion, tilde expansion, or shell globbing. Plus it is now vulnerable to shell injection (not that this is the easiest way to mess with someone's computer and make them run malicious commands, but still, it's the theory of the thing...)

Last edited by eschwartz; 02-03-2016 at 11:01 AM.
eschwartz is offline   Reply With Quote
Old 02-03-2016, 11:02 AM   #21
Doitsu
Wizard
Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.Doitsu ought to be getting tired of karma fortunes by now.
 
Doitsu's Avatar
 
Posts: 4,580
Karma: 14563843
Join Date: Dec 2010
Device: Kindle PW2
Quote:
Originally Posted by eschwartz View Post
Again, it works fine once I fixed the plugin.

The problem is that subprocess.Popen expects either:
  1. a list, and shell=False (shell defaults to false)
  2. a string, and shell=True
To be honest, I can't follow you. Can you please post the lines that you've changed?
Doitsu is offline   Reply With Quote
Old 02-03-2016, 11:11 AM   #22
eschwartz
Ex-Helpdesk Junkie
eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.eschwartz ought to be getting tired of karma fortunes by now.
 
eschwartz's Avatar
 
Posts: 19,287
Karma: 83106505
Join Date: Nov 2012
Location: The Beaten Path, USA, Roundworld, This Side of Infinity
Device: Kindle Touch fw5.3.7 (Wifi only)
Code:
def OptiWrapper(*args):
    # execute optimizer program
    # list(args) is a no-op, since it is already a list...
    ret = call(list(args), shell=True)
    return ret
This is BAD.

args is a list, and shell=True

These conditions cannot both be true.


Either use:
Code:
def OptiWrapper(*args):
    # execute optimizer program
    ret = call(args)
    return ret
or
Code:
def OptiWrapper(*args):
    # execute optimizer program
    # this actually requires a lot more checking of proper shell
    # quoting/escaping than I display here
    ret = call(' '.join(args), shell=True)
    return ret

See the python docs: https://docs.python.org/3/library/su...en-constructor

Quote:
On POSIX with shell=True, the shell defaults to /bin/sh. If args is a string, the string specifies the command to execute through the shell. This means that the string must be formatted exactly as it would be when typed at the shell prompt. This includes, for example, quoting or backslash escaping filenames with spaces in them. If args is a sequence, the first item specifies the command string, and any additional items will be treated as additional arguments to the shell itself. That is to say, Popen does the equivalent of:
Code:
Popen(['/bin/sh', '-c', args[0], args[1], ...])

EDIT: useless nit #2
Why define a special function as a wrapper for subprocess.call, when it is used only once and adds no particular value other than turning on the shell and doing list --> list conversion?
It was staring me in the face while looking at the source of the hanging...

Last edited by eschwartz; 02-03-2016 at 11:22 AM.
eschwartz is offline   Reply With Quote
Old 02-03-2016, 12:44 PM   #23
Toxaris
Wizard
Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.Toxaris ought to be getting tired of karma fortunes by now.
 
Toxaris's Avatar
 
Posts: 4,521
Karma: 121692313
Join Date: Oct 2009
Location: Heemskerk, NL
Device: PRS-300, PRS-T1
The shell=True is indeed a mistake which was entered during the testing process. I apparently forgot to remove it. On Windows it does not cause an issue...

A small update where this is corrected.

The reason why I used a sub for the wrapper is simple. It is easier for updates if this is already split. I agree that it seems silly for one command only, but that is the current situation.
Toxaris is offline   Reply With Quote
Reply

Thread Tools Search this Thread
Search this Thread:

Advanced Search

Forum Jump


All times are GMT -4. The time now is 03:50 PM.


MobileRead.com is a privately owned, operated and funded community.