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

convert: Respect album_art_maxwidth in conjunction with embed #2116

Closed
broddo opened this issue Jul 5, 2016 · 2 comments · Fixed by #4273
Closed

convert: Respect album_art_maxwidth in conjunction with embed #2116

broddo opened this issue Jul 5, 2016 · 2 comments · Fixed by #4273
Labels
feature features we would like to implement

Comments

@broddo
Copy link
Contributor

broddo commented Jul 5, 2016

Problem

Hey guys, I think I've spotted a bug - like the title says. I noticed that my converted files have the full size images rather than my requested maxwidth of 600.

However, if I just use beet embedart QUERY, the resize works fine and I can see the temporary images of the correct resolution being created in /tmp. So this tells me that its not a problem with imagemagick.

$ beet -vv convert AURORA runaway

Led to this problem:

user configuration: /home/barry/.config/beets/config.yaml
data directory: /home/barry/.config/beets
plugin paths: 
Sending event: pluginload
artresizer: method is (2, (6, 9, 4))
library database: /home/barry/.config/beets/musiclibrary.blb
library directory: /home/barry/Music/lossless
Sending event: library_opened
AURORA - All My Demons Greeting Me as a Friend - Runaway
Convert? (Y/n) Y
convert: Encoding /home/barry/Music/lossless/AURORA/All My Demons Greeting Me as a Friend/01 Runaway.flac
convert: Finished encoding /home/barry/Music/lossless/AURORA/All My Demons Greeting Me as a Friend/01 Runaway.flac
Sending event: write
Sending event: after_write
convert: embedding album art from /home/barry/Music/lossless/AURORA/All My Demons Greeting Me as a Friend/cover.jpg
convert: embedding /home/barry/Music/lossless/AURORA/All My Demons Greeting Me as a Friend/cover.jpg
Sending event: write
Sending event: after_write
Sending event: after_convert
Sending event: cli_exit

I would have expected to see a line like:

artresizer: ImageMagick resizing /home/barry/Music/lossless/AURORA/All My Demons Greeting Me as a Friend/cover.jpg to /tmp/tmp5vch3h.jpg

Setup

  • OS: Arch Linux
  • Python version: 2.7.12
  • beets version: 1.3.18
  • Turning off plugins made problem go away (yes/no): no

My configuration (output of beet config) is:

convert:
    never_convert_lossy_files: yes
    dest: ~/Music/lossy
    embed: yes
    album_art_maxwidth: 600
    format: aac
    formats:
        aac:
            command: ffmpeg -i $source -y -c:a libfdk_aac -vbr 3 $dest
            extension: m4a
@sampsyo sampsyo added needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." feature features we would like to implement and removed needinfo We need more details or follow-up from the filer before this can be tagged "bug" or "feature." labels Jul 5, 2016
@sampsyo sampsyo changed the title Convert plugin with embed option not respecting album_art_maxwidth convert: Respect album_art_maxwidth in conjunction with embed Jul 5, 2016
@sampsyo
Copy link
Member

sampsyo commented Jul 5, 2016

Thanks for the report! I took a look at the code, and it seems that album_art_maxwidth is currently only respected when copying album art files (i.e., with the copy_album_art option). It doesn't apply when embedding album art during conversion (i.e., with the embed option). It seems reasonable to me to make it apply to both.

Technical details: we use the setting in the convert_album_art method, but not in the condition under if self.config['embed']:. The right fix is probably just to pass the maxwidth parameter to the embed_item call therein.

@JDLH
Copy link

JDLH commented Jul 18, 2016

This is a problem for me also. It seems my ancient Android phone will display album art embedded in MP3 files if 300x300 pixels, but not if 450x450 pixels. Thus this bug is blocking me from getting my MP3s right for that device. I may come up with a patch for this soon.

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants