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

import.from_scratch clears the `format` field => replaygain uses wrong tag format #2972

Closed
zsinskri opened this issue Jun 30, 2018 · 4 comments

Comments

Projects
None yet
3 participants
@zsinskri
Copy link
Contributor

commented Jun 30, 2018

Problem

original Problem:

All tracks in my library (Opus and FLAC) have REPLAYGAIN_{ALBUM,TRACK}_{GAIN,PEAK} tags and no R128_{ALBUM,TRACK}_GAIN tags after running beet replaygain -a, even the Opus files.
I tried gstreamer and bs1770gain replaygain backends.
With bs1770gain I also tried explicitly setting method: ebu. and setting r128: Opus FLAC.

Running this command in verbose (-vv) mode:

$ beet -vv replaygain LITE Cubic Else
user configuration: /home/zsin/.config/beets/config.yaml
data directory: /home/zsin/.config/beets
plugin paths: 
Sending event: pluginload
lyrics: Disabling google source: no API key configured.
/home/zsin/incoming/tmp/beets/beets/plugins.py:130: DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() or inspect.getfullargspec()
  argspec = inspect.getargspec(func)
/home/zsin/incoming/tmp/beets/beets/plugins.py:130: DeprecationWarning: inspect.getargspec() is deprecated, use inspect.signature() or inspect.getfullargspec()
  argspec = inspect.getargspec(func)
library database: /home/zsin/.local/share/beets/library.db
library directory: /home/zsin/music
Sending event: library_opened
replaygain: analyzing LITE - Cubic - 01 Else
replaygain: executing bs1770gain --ebu --xml -p /home/zsin/music/LITE/Cubic/01 Else.opus
replaygain: analysis finished: <bs1770gain>
  <album>
    <track total="1" number="1" file="01&#x20;Else&#x2E;opus">
      <integrated lufs="-10.96" lu="-12.04" />
      <sample-peak spfs="1.71" factor="1.217485" />
    </track>
    <summary total="1">
      <integrated lufs="-10.96" lu="-12.04" />
      <sample-peak spfs="1.71" factor="1.217485" />
    </summary>
  </album>
</bs1770gain>

replaygain: 1 items, 1 results
Sending event: database_change
replaygain: applied track gain -12.04, peak 1.217485
Sending event: write
Sending event: after_write
Sending event: cli_exit

Note the deprecation warnings mentioned in #2826 They go away when disabling plugins. And without plugins item.format is still "" (see below), so I do not believe them to be the problem.

Led to this problem:

$ ffmpeg -i ~/music/LITE/Cubic/01\ Else.opus 2>&1 | grep -iE 'gain|r128'
      REPLAYGAIN_ALBUM_GAIN: -12.55 dB
      REPLAYGAIN_ALBUM_PEAK: 1.244816
      REPLAYGAIN_TRACK_GAIN: -12.04 dB
      REPLAYGAIN_TRACK_PEAK: 1.217485
$ ffmpeg -i ~/music/LITE/Cubic/01\ Else.opus 2>&1 | grep -iE 'input|audio'
Input #0, ogg, from '/home/zsin/music/LITE/Cubic/01 Else.opus':
    Stream #0:0(eng): Audio: opus, 48000 Hz, stereo, fltp

Trying to analyse the problem

Adding some print statements to replaygain (cloned from master):

    def should_use_r128(self, item):
        """Checks the plugin setting to decide whether the calculation
        should be done using the EBU R128 standard and use R128_ tags instead.
        """
        print(f"should_use_r128: {repr(item.format)} in {self.r128_whitelist} ==> {item.format in self.r128_whitelist}")
        return item.format in self.r128_whitelist
b replaygain LITE Cubic Else
replaygain: analyzing LITE - Cubic - 01 Else
should_use_r128: '' in ['Opus', 'FLAC'] ==> False

item.field is the empty string.

$ # with all plugins disabled (enabling them just adds the deprecation warnings):
$ beet ls -f '$title [$format]' LITE Cubic Else
Else []

Inserting a breakpoint into list to continue debugging without plugins:

def list_items(lib, query, album, fmt=u''):
    """Print out items in lib matching query. If album, then search for
    albums instead of single items.
    """
    if album:
        for album in lib.albums(query):
            ui.print_(format(album, fmt))
    else:
        for item in lib.items(query):
            import pdb; pdb.set_trace()
            ui.print_(format(item, fmt))
$ beet ls LITE Cubic Else
> /home/zsin/incoming/tmp/beets/beets/ui/commands.py(1068)list_items()
-> ui.print_(format(item, fmt))
(Pdb) p item.format
''
(Pdb) item.read()
(Pdb) p item.format
'Opus'
(Pdb) 

So the MediaFile correctly identifies the format, but it is never actually read.
The info plugin uses MediaFile directly:

$ beet info LITE Cubic Else | grep format
              format: Opus

Problem

I was not able to find out where format is supposed to be read into the Item for use by list and replaygain. Could someone please provide a hint on that? I'd like to test if/where there is problem on my side.

Setup

  • OS: Fedora 28
  • Python version: 3.6.5 (also partially tried 2.7.15 with same results)
  • beets version: 1.4.8 (from current git master (commit 0c033bd))
    I observed the same symptom (no R128_* tags on Opus) with beets 1.4.6 (installed by Fedora's package manager).
  • Turning off plugins made problem go away (yes/no):

Configuration without plugins

My configuration (output of beet config) is:

directory: ~/music
library: ~/.local/share/beets/library.db
asciify_paths: yes

import:
    from_scratch: yes
    languages: en

paths:
    singleton: $artist/$title

format_item: $artist - $album - $track $title

Configuration (with bs1770 and explicit options)

My configuration (output of beet config) is:

 lyrics:
    bing_lang_from: []
    auto: yes
    bing_client_secret: REDACTED
    bing_lang_to:
    google_API_key: REDACTED
    google_engine_ID: REDACTED
    genius_api_key: REDACTED
    fallback:
    force: no
    local: no
    sources:
    - google
    - lyricwiki
    - musixmatch
    - genius
directory: ~/music
library: ~/.local/share/beets/library.db
asciify_paths: yes

import:
    from_scratch: yes
    languages: en

paths:
    singleton: $artist/$title

format_item: $artist - $album - $track $title

plugins:
- fromfilename
- edit
- fetchart
- ftintitle
- lastgenre
- lyrics
- replaygain
- convert
- fuzzy
- info
- mbsubmit
- mbsync
- missing
lastgenre:
    count: 5
    prefer_specific: no
    source: track
    whitelist: yes
    min_weight: 10
    fallback:
    canonical: no
    force: yes
    auto: yes
    separator: ', '
replaygain:
    overwrite: yes
    backend: bs1770gain
    r128: Opus FLAC
    method: ebu
    auto: yes
    targetlevel: 89
    chunk_at: 5000
convert:
    copy_album_art: yes
    embed: no
    dest: ~/converted_music
    format: opus
    formats:
        opus: ffmpeg-audio-convert-or-copy $source $dest opus -y -vn -ab 96k
        aac:
            command: ffmpeg -i $source -y -vn -acodec aac -aq 1 $dest
            extension: m4a
        alac:
            command: ffmpeg -i $source -y -vn -acodec alac $dest
            extension: m4a
        flac: ffmpeg -i $source -y -vn -acodec flac $dest
        mp3: ffmpeg -i $source -y -vn -aq 2 $dest
        ogg: ffmpeg -i $source -y -vn -acodec libvorbis -aq 3 $dest
        wma: ffmpeg -i $source -y -vn -acodec wmav2 -vn $dest
    pretend: no
    threads: 4
    max_bitrate: 500
    auto: no
    tmpdir:
    quiet: no

    paths: {}
    no_convert: ''
    never_convert_lossy_files: no
    album_art_maxwidth: 0
fuzzy:
    prefix: '~'
    threshold: 0.7
missing:
    count: no
    total: no
    album: no
fetchart:
    auto: yes
    minwidth: 0
    maxwidth: 0
    enforce_ratio: no
    cautious: no
    cover_names:
    - cover
    - front
    - art
    - album
    - folder
    sources:
    - filesystem
    - coverart
    - itunes
    - amazon
    - albumart
    google_key: REDACTED
    google_engine: 001442825323518660753:hrh5ch1gjzm
    fanarttv_key: REDACTED
    store_source: no
edit:
    albumfields: album albumartist
    itemfields: track title artist album
    ignore_fields: id path
ftintitle:
    auto: yes
    drop: no
    format: feat. {0}
mbsubmit:
    format: $track. $title - $artist ($length)
    threshold: medium
@zsinskri

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2018

I just noticed I have the option import.from_scratch: yes. Disabling it and reimporting the library seems to completely fix the issue.

So reformulating the issue (GitHub does support changing the title, doesn't it? I'll try to update it accordingly…):

Problem

When import.from_scratch is enabled the format field of Item is cleared and never set. Thus replaygain fails to correctly identify the format and thus fails to use R128_{ALBUM,TRACK}_GAIN tags when it should.
Also ls -f '$format' yields just the empty string.

@zsinskri zsinskri changed the title no R128_* tags / item.format is empty string import.from_scratch clears the `format` field => replaygain uses wrong tag format Jun 30, 2018

@sampsyo sampsyo added the bug label Jun 30, 2018

@sampsyo

This comment has been minimized.

Copy link
Member

commented Jun 30, 2018

Thank you for pointing this out! It does seem like the from_scratch option should not remove any of the immutable file-based fields like format or bitrate.

@tummychow implemented that option in #2755. Could you please take a look, @tummychow?

@tummychow

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2018

ah, it probably needs to use this here. I can fix this myself in a few days if nobody else gets to it sooner.

zsinskri added a commit to zsinskri/beets that referenced this issue Jul 1, 2018

from_scratch import: only remove writable fields
When importing with the configuration option ``from_scratch`` set, only remove
writable fields from library. E.g. keep fields like ``format`` and ``bitrate``.

This fixes beetbox#2972.
@sampsyo

This comment has been minimized.

Copy link
Member

commented Jul 2, 2018

Indeed; that looks like the right thing.

@sampsyo sampsyo closed this in #2975 Jul 2, 2018

sampsyo added a commit that referenced this issue Jul 2, 2018

Merge pull request #2975 from zsinskri/2972-from-scratch-keep-immutable
 from_scratch option: keep immutable fields (#2972)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.