beets icon indicating copy to clipboard operation
beets copied to clipboard

convert: Paths should reflect the new file, not the original file

Open richardpowellus opened this issue 10 years ago • 14 comments

Hi,

When I use convert I have the output path set as, for example, $format. The issue is that even though the output file being created is of format AAC the file is being placed in the output path FLAC (assuming the input format is FLAC).

In summary, when using convert the $format variable in the paths configuration option for convert should use the output file's $format and not the input file's $format.

richardpowellus avatar Mar 17 '15 03:03 richardpowellus

Hi, could you provide the command you use (with -vv flag) s well as your config? The more info the better.

brunal avatar Mar 17 '15 13:03 brunal

Here's my beets config.yaml:

library: /mnt/nas/music/.beets/library.db
directory: /mnt/nas/music
plugins: fetchart embedart convert
import:
    move: yes
paths:
    default: $albumartist/$album [$year]/$format/$artist - $track - $title
    singleton: Non-Album/$artist/$format/$artist - $title
    comp: Compilations/$album [$year]/$format/$artist - $track - $title
convert:
    dest: /mnt/nas/music
#    paths:
#        default: $albumartist/$album [$year]/AAC/$artist - $track - $title
#        singleton: Non-Album/$artist/AAC/$artist - $title
#        comp: Compilations/$album [$year]/AAC/$artist - $track - $title
    never_convert_lossy_files: true
    format: aac
    formats:
        aac:
            command: ffmpeg -i $source -y -c:a libfdk_aac -vbr 5 $dest
            extension: m4a

Here's the output of: beet convert --pretend title:'Let It Go' format:'flac' track:5

ffmpeg -i '/mnt/nas/music/Kristen Anderson-Lopez, Robert Lopez & Christophe Beck/Frozen [2013]/FLAC/Kristen Anderson-Lopez, Robert Lopez & Christophe Beck - 05 - Let It Go.flac' -y -c:a libfdk_aac -vbr 5 '/mnt/nas/music/Kristen Anderson-Lopez, Robert Lopez & Christophe Beck/Frozen [2013]/FLAC/Kristen Anderson-Lopez & Robert Lopez - 05 - Let It Go.m4a'

My expectation here is that the output file is placed in a subfolder called "AAC" and not "FLAC" since the output file is not a FLAC file.

richardpowellus avatar Mar 17 '15 17:03 richardpowellus

One other option would be for the convert plug-in to expose $convert.format. That way I could specify the default path as: $albumartist/$album [$year]/$convert.format/$artist - $track - $title

In fact I like this better because then I could define an aac-lq and aac-hq and have the respective folders created when I do the convert.

richardpowellus avatar Mar 17 '15 17:03 richardpowellus

Agreed; we should make the path templates reflect the newly converted file, not the old file. This currently doesn't happen because we run the encoding command with the destination path as a parameter, so the destination has to be generated before we have the new file.

This isn't just about $format but also about $bitrate, etc.

I could have sworn this came up before, but I can't find another issue about it or a mailing list post. Must have been on IRC or elsewhere.

sampsyo avatar Mar 18 '15 02:03 sampsyo

You're right, this has come up before. I just scrubbed through the issues and it looks like you guys discussed this in issue #744.

richardpowellus avatar Mar 18 '15 17:03 richardpowellus

Aha, thanks! A discussion in another bug's thread: that's why I couldn't find it quickly.

sampsyo avatar Mar 18 '15 19:03 sampsyo

Any word on this? :-)

sdfg2 avatar Mar 15 '16 18:03 sdfg2

@sdfg2: As a general rule, there's no news unless it's posted here. Please feel free to help out!

sampsyo avatar Mar 15 '16 19:03 sampsyo

Quote from https://github.com/beetbox/beets/issues/1360#issuecomment-82683353

Agreed; we should make the path templates reflect the newly converted file, not the old file. This currently doesn't happen because we run the encoding command with the destination path as a parameter, so the destination has to be generated before we have the new file.

This isn't just about $format but also about $bitrate, etc.

I disagree on the conclusion. Target $bitrate can't be reliably obtained: let's think of a case of using custom scripts for converting or about aliases in the shell. Period. Unlike that, target format is known right away from the config file when we call convert. And this can be potentially exploited.

I've checked the call stack of the convert process and not a surprise it's a long way to template evaluation for each converted item. Target extension is used only to replace source one once full destination file path is already known (replace_ext()): https://github.com/beetbox/beets/blob/d35a68d0d8cfa4b1481365f5bee8ae2bc0c96dbf/beetsplug/convert.py#L267-L276 From my limited familiarity with beets' architecture, the best idea I have so far is to have a new template function like %convert{$format, $target_extension}. Plugins are optional and we shouldn't affect core functionality in an ugly manner just to solve this issue. So such template could be just a generic replace - %replace{$format, str}.

Anyway would be nice if it gets a fresh look.

I'll check if %if{condition,truetext,falsetext} could be used as a workaround.

murlakatamenka avatar Jan 16 '19 20:01 murlakatamenka

It is true that $format could be determined before the conversion while $bitrate cannot; good point! But it's also possible—and perhaps a simpler change—for us to just move the entire destination computation until after the file conversion is done, which would affect both without much additional effort.

sampsyo avatar Jan 16 '19 20:01 sampsyo

Oops, sorry for the duplicate! I swear I searched before opening issue #3407, but obviously I didn't search very well.

Just for the record, I would argue that this is not a feature request, but a bug. I can certainly see that it would be a low-priority bug, but... templates to use the file-format in the path during import are already part of beets; the option to convert files during import is already part of beets. When you use these two pieces of existing functionality in conjunction, the result is simply wrong.

Does that make sense?

scruloose avatar Oct 18 '19 11:10 scruloose

Not sure if I am running into the same issue, but I have the convert plugin enabled and configured to automatically create an ALAC version of imported files. What is happening is that the ALAC files are being written to /music/in, which is where the original imported files should be, instead of to /music/ALAC - and the originally imported format is missing. Nothing appears in /music/ALAC.

Full configuration file:

directory: /music/in
art_filename: folder

ui:
  color: yes

import:
  move: yes
  timid: yes
  languages: en
  bell: yes

match:
  strong_rec_thresh: 0.02
  ignored_media: ['Data CD', 'DVD', 'DVD-Video', 'Blu-ray', 'HD-DVD',
                  'VCD', 'SVCD', 'UMD', 'VHS']

#replace:
#  '[\’]': ''
#  '[…]': ...

plugins: chroma discogs embedart ftintitle scrub zero info replaygain convert
# replaygain edit

convert:
  auto: yes
  album_art_maxwidth: 800
  dest: /music/ALAC
  embed: yes
  never_convert_lossy_files: yes
  format: alac

embedart:
  auto: no

ftintitle:
  auto: yes
  format: (feat. {0})

replaygain:
  auto: yes
  backend: gstreamer
  overwrite: yes
  noclip: yes

scrub:
  auto: yes

zero:
  auto: yes
  fields: images

chroma:
  auto: yes

ghost avatar Dec 29 '19 18:12 ghost

@Barracuda6: no, that's not this problem. The plugin's auto option puts the converted files in your music directory, which you have configured here:

directory: /music/in

The dest option is ignored in auto mode. Converted files are added to your library. See #1840 for a request to invert this logic.

sampsyo avatar Dec 29 '19 19:12 sampsyo

Oh, okay - thank you for the clarification.

ghost avatar Dec 30 '19 00:12 ghost