beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.77k stars 1.82k forks source link

Use explicit paths to ImageMagick tools on Windows #2093

Closed Schweinepriester closed 5 years ago

Schweinepriester commented 8 years ago

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

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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: https://github.com/beetbox/beets/issues/670#issuecomment-225279659

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

sampsyo commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

:smiley: 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 commented 5 years ago

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