Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use explicit paths to ImageMagick tools on Windows #2093

Closed
Schweinepriester opened this issue Jun 28, 2016 · 10 comments · Fixed by #3236
Closed

Use explicit paths to ImageMagick tools on Windows #2093

Schweinepriester opened this issue Jun 28, 2016 · 10 comments · Fixed by #3236
Labels
feature features we would like to implement windows Relates specifically to Windows OS

Comments

@Schweinepriester
Copy link
Contributor

ImageMagick/artresizer fails on windows?

PS C:\Users\Kai\Desktop> beet -vv import '.\Space Probe Taurus - 15.rar'
…
fetchart: trying source coverart for album Space Probe Taurus - Mondo Satan
ImageMagick check `convert --version` failed: Command 'convert --version' returned non-zero exit status 4
artresizer: method is (3, 0)
…
PS C:\Users\Kai> convert --version
Version: ImageMagick 7.0.2-1 Q16 x64 2016-06-23 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2015 ImageMagick Studio LLC
License: http://www.imagemagick.org/script/license.php
Visual C++: 180040629
Features: Cipher DPC Modules OpenMP
Delegates (built-in): bzlib cairo flif freetype jng jp2 jpeg lcms lqr openexr pangocairo png ps rsvg tiff webp xml zlib

Setup

  • OS: Windows 10
  • Python version: Python 2.7.11 (v2.7.11:6d1b6a68f775, Dec 5 2015, 20:32:19) [MSC v.1500 32 bit (Intel)] on win32
  • beets version: 1.3.19
  • Turning off plugins made problem go away (yes/no):

My configuration (output of beet config) is:

PS C:\Users\Kai\Desktop> beet config
fetchart: The `fetch_art.remote_priority` configuration option has been deprecated, see the documentation.
lyrics:
    bing_lang_from: []
    auto: yes
    force: no
    bing_client_secret: REDACTED
    google_API_key: REDACTED
    google_engine_ID: REDACTED
    sources:
    - google
    - lyricwiki
    - lyrics.com
    - musixmatch
    fallback:
    genius_api_key: REDACTED
    bing_lang_to:

paths:
    default: $albumartist/$album%aunique{}/%if{$multidisc,CD$disc/}$albumartist - $album%aunique{} - $track $title
item_fields:
    multidisc: 1 if disctotal > 1 else 0
per_disc_numbering: yes
fetchart:
    auto: yes
    minwidth: 750
    maxwidth: 2000
    enforce_ratio: yes
    remote_priority: yes
    sources: coverart itunes amazon albumart filesystem
    google_engine: 001442825323518660753:hrh5ch1gjzm
    cautious: no
    store_source: no
    google_key: REDACTED
    fanarttv_key: REDACTED
    cover_names:
    - cover
    - front
    - art
    - album
    - folder
format_album: 'Musik: $albumartist - $album ($genre - $year) @ $bitrate kBit/s'
library: E:\beets\musiclibrary.db

replace:
    '[\\/]': _
    ^\.: ''
    '[\x00-\x1f]': _
    '[<>:"\?\*\|]': ''
    \.$: _
    \s+$: ''
    ^\s+: ''
ftintitle:
    auto: yes
    drop: no
    format: feat. {0}
zero:
    fields: comments
    update_database: no
    keep_fields: []
embedart:
    auto: yes
    remove_art_file: yes
    compare_threshold: 0
    ifempty: no
    maxwidth: 0

plugins: fetchart lastgenre embedart zero inline lyrics ftintitle discogs fromfilename edit
directory: F:\Audio\Mediathek\Musik

import:
    copy: yes
    write: yes
    resume: ask
    quiet_fallback: skip
    timid: no
    log: beetslog.txt
album_fields:
    bitrate: "total = 0\nfor item in items:\n    total += item.bitrate\nbitrate = total / len(items)\nreturn str(bitrate
)[:3]\n"
discogs:
    tokenfile: discogs_token.json
    apikey: REDACTED
    apisecret: REDACTED
    source_weight: 0.5
edit:
    itemfields: track title artist album
    albumfields: album albumartist
    ignore_fields: id path
lastgenre:
    count: 1
    source: album
    force: yes
    min_weight: 10
    auto: yes
    whitelist: yes
    separator: ', '
    fallback:
    canonical: no
pathfields: {}
@jackwilsdon
Copy link
Member

jackwilsdon commented Jun 28, 2016

Do you think you could provide the return code from convert --version in PowerShell? Run this after convert --version to get the exit code:

Write-Host "Code: $LASTEXITCODE"

@Schweinepriester
Copy link
Contributor Author

Sure! :)

PS C:\Users\Kai> convert --version
Version: ImageMagick 7.0.2-1 Q16 x64 2016-06-23 http://www.imagemagick.org
Copyright: Copyright (C) 1999-2015 ImageMagick Studio LLC
License: http://www.imagemagick.org/script/license.php
Visual C++: 180040629
Features: Cipher DPC Modules OpenMP
Delegates (built-in): bzlib cairo flif freetype jng jp2 jpeg lcms lqr openexr pangocairo png ps rsvg tiff webp xml zlib
PS C:\Users\Kai> Write-Host "Code: $LASTEXITCODE"
Code: 0

@sampsyo sampsyo added the feature features we would like to implement label Jun 29, 2016
@sampsyo sampsyo changed the title ImageMagick/artresizer fails on windows? Use explicit paths to ImageMagick tools on Windows Jun 29, 2016
@sampsyo
Copy link
Member

sampsyo commented Jun 29, 2016

I ran into this while experimenting with ImageMagick on Windows recently. The problem is that Windows has a convert.exe that shadows IM's convert.exe program. There's more background in this thread: #670 (comment)

And there's also a relevant SO answer: http://stackoverflow.com/questions/15016974/running-imagemagick-convert-console-application-from-python

Alas, the behavior from inside beets doesn't necessarily match the behavior when typing convert in PowerShell. I wasn't able to find a way to make this work out of the box, so I think the only solution is to add a configuration option to beets to let you specify full paths to the IM tools. Unless anyone else has a better idea?

@Schweinepriester
Copy link
Contributor Author

What I didnt get from reading the thread is: What has changed to break this? Since it seemed to work in the past (1.3.16, .17 probably too)?

@sampsyo
Copy link
Member

sampsyo commented Jun 29, 2016

Is there any chance it was failing silently before? In previous versions, we could be erroneously convinced that we had a working convert and we'd just go ahead and try to use it -- which would fail every time.

@Schweinepriester
Copy link
Contributor Author

There is a chance - how can we be sure? Place a large art file locally and force resizing using <1.3.18?

@sampsyo
Copy link
Member

sampsyo commented Jun 30, 2016

That seems logical -- but I'm reasonably confident that's what's going on. I can't see any other way around this aside from using a full, explicit path to the convert executable.

@jackwilsdon
Copy link
Member

jackwilsdon commented Jul 1, 2016

@sampsyo: I don't know how I forgot about my replies in #670 😆. Using absolute paths does seem to be the best way 😕. I still don't understand why cmd.exe prefers the built in convert.exe (when using shell=True) instead of the one provided by ImageMagick? Is it not %PATH% for some reason?

@sampsyo
Copy link
Member

sampsyo commented Jul 1, 2016

😃 Good point! You were the one who originally pointed this out.

I think the deal is that using shell=False, which we do for sanity reasons, doesn't use the Windows %PATH% at all. I admittedly don't understand this very well, but I think this SO answer indicates that you only get executables in the current working directory and builtins: http://stackoverflow.com/a/5659249

On the other hand, while searching for details, I found this other SO answer that seems to indicate that the problem is just that %PATH% doesn't get inherited by the invocation by default. Explicitly threading it through might work: http://stackoverflow.com/a/14679560

That report seems a little unreliable, but perhaps worth a try?

@sampsyo
Copy link
Member

sampsyo commented Apr 30, 2019

Fixed in #3236 by using the magick executable when it's available. Presumably, Windows doesn't have a built-in, conflicting magick.exe. 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature features we would like to implement windows Relates specifically to Windows OS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants