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

zsinskri commented 6 years ago

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 0c033bdaf10b78be04e6ebc8f998d9d3d5f70e3d)) 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 commented 6 years ago

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.

sampsyo commented 6 years ago

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

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

sampsyo commented 6 years ago

Indeed; that looks like the right thing.

beetbox / beets