replaygain: R128_ALBUM_GAIN should use a Q7.8 number

[zsinskri]

Since commit 31326eb beets stores the R128_ALBUM_GAIN formatted as a float, not as a Q7.8, as required by IETF RFC 7845, 5.3.1.

Problem

Running this command in verbose (-vv) mode on master:

$ beet -vv replaygain -af "Anne Clark"
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.
lastgenre: Loading canonicalization tree /home/zsin/.local/lib/beets/beetsplug/lastgenre/genres-tree.yaml
library database: /home/zsin/music/beets-library.db
library directory: /home/zsin/music
Sending event: library_opened
replaygain: analyzing Anne Clark - Joined Up Writing
replaygain: executing bs1770gain --ebu --xml -p /home/zsin/music/Anne Clark/Joined Up Writing/01 Nothing at All.opus /home/zsin/music/Anne Clark/Joined Up Writing/02 Weltschmerz.opus /home/zsin/music/Anne Clark/Joined Up Writing/03 Killing Time.opus /home/zsin/music/Anne Clark/Joined Up Writing/04 True Love Tales.opus /home/zsin/music/Anne Clark/Joined Up Writing/05 Self Destruct.opus /home/zsin/music/Anne Clark/Joined Up Writing/06 Our Darkness.opus
replaygain: analysis finished: <bs1770gain>
  <album>
    <track total="6" number="1" file="01&#x20;Nothing&#x20;at&#x20;All&#x2E;opus">
      <integrated lufs="-17.04" lu="-5.96" />
      <sample-peak spfs="-1.03" factor="0.887686" />
    </track>
    <track total="6" number="2" file="02&#x20;Weltschmerz&#x2E;opus">
      <integrated lufs="-17.50" lu="-5.50" />
      <sample-peak spfs="-0.44" factor="0.950337" />
    </track>
    <track total="6" number="3" file="03&#x20;Killing&#x20;Time&#x2E;opus">
      <integrated lufs="-18.15" lu="-4.85" />
      <sample-peak spfs="-0.27" factor="0.969098" />
    </track>
    <track total="6" number="4" file="04&#x20;True&#x20;Love&#x20;Tales&#x2E;opus">
      <integrated lufs="-16.66" lu="-6.34" />
      <sample-peak spfs="-1.21" factor="0.869624" />
    </track>
    <track total="6" number="5" file="05&#x20;Self&#x20;Destruct&#x2E;opus">
      <integrated lufs="-15.73" lu="-7.27" />
      <sample-peak spfs="0.05" factor="1.006166" />
    </track>
    <track total="6" number="6" file="06&#x20;Our&#x20;Darkness&#x2E;opus">
      <integrated lufs="-15.56" lu="-7.44" />
      <sample-peak spfs="0.47" factor="1.055847" />
    </track>
    <summary total="6">
      <integrated lufs="-16.48" lu="-6.52" />
      <sample-peak spfs="0.47" factor="1.055847" />
    </summary>
  </album>
</bs1770gain>

replaygain: 6 items, 7 results
Sending event: database_change
replaygain: applied r128 track gain -1526
Sending event: database_change
replaygain: applied r128 album gain -6.52
Sending event: write
zero: images:  -> None
zero: rg_track_gain: None -> None
zero: rg_album_gain: None -> None
zero: rg_track_peak: None -> None
zero: rg_album_peak: None -> None
/usr/lib/python3.7/site-packages/mutagen/apev2.py:35: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
  from collections import MutableSequence
Sending event: after_write
Sending event: database_change
replaygain: applied r128 track gain -1408
Sending event: database_change
replaygain: applied r128 album gain -6.52
Sending event: write
zero: images:  -> None
zero: rg_track_gain: None -> None
zero: rg_album_gain: None -> None
zero: rg_track_peak: None -> None
zero: rg_album_peak: None -> None
Sending event: after_write
Sending event: database_change
replaygain: applied r128 track gain -1242
Sending event: database_change
replaygain: applied r128 album gain -6.52
Sending event: write
zero: images:  -> None
zero: rg_track_gain: None -> None
zero: rg_album_gain: None -> None
zero: rg_track_peak: None -> None
zero: rg_album_peak: None -> None
Sending event: after_write
Sending event: database_change
replaygain: applied r128 track gain -1623
Sending event: database_change
replaygain: applied r128 album gain -6.52
Sending event: write
zero: images:  -> None
zero: rg_track_gain: None -> None
zero: rg_album_gain: None -> None
zero: rg_track_peak: None -> None
zero: rg_album_peak: None -> None
Sending event: after_write
Sending event: database_change
replaygain: applied r128 track gain -1861
Sending event: database_change
replaygain: applied r128 album gain -6.52
Sending event: write
zero: images:  -> None
zero: rg_track_gain: None -> None
zero: rg_album_gain: None -> None
zero: rg_track_peak: None -> None
zero: rg_album_peak: None -> None
Sending event: after_write
Sending event: database_change
replaygain: applied r128 track gain -1905
Sending event: database_change
replaygain: applied r128 album gain -6.52
Sending event: write
zero: images:  -> None
zero: rg_track_gain: None -> None
zero: rg_album_gain: None -> None
zero: rg_track_peak: None -> None
zero: rg_album_peak: None -> None
Sending event: after_write
Sending event: cli_exit

Led to this problem:

$ ffmpeg -i 01\ Nothing\ at\ All.opus -f null - 2>&1 | grep GAIN
      R128_ALBUM_GAIN : -6.52
      R128_TRACK_GAIN : -1526
[…]

On commit 1619761 the same command formats the value in R128_ALBUM_GAIN differently:

$ ffmpeg -i 01\ Nothing\ at\ All.opus -f null - 2>&1 | grep GAIN
      R128_ALBUM_GAIN : -1669
      R128_TRACK_GAIN : -1526
[…]

As far as I can determine, the old behaviour is correct, as it conforms to the aforementioned RFC.

Setup

• OS: GNU/Linux
• Python version: CPython 3.7.8
• beets version: master
• Turning off plugins made problem go away (yes/no): Problem does not exist without replaygain plugin.

The relevant part of my configuration is:

replaygain:
  #backend: ffmpeg
  backend: bs1770gain
  peak: sample
  r128: Opus FLAC

(The unmerged ffmpeg backend was tested aswell.)
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: ~/music/beets-library.db
asciify_paths: yes

paths:
    singleton: $artist/$title

import:
    languages: en
    detail: yes

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

plugins:
- convert
- edit
- fetchart
- fromfilename
- ftintitle
- fuzzy
- info
- lastgenre
- lyrics
- mbsync
- missing
- replaygain
- zero
convert:
    copy_album_art: yes
    embed: no
    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
    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
lastgenre:
    count: 5
    prefer_specific: yes
    canonical: yes
    source: track
    whitelist: yes
    min_weight: 10
    fallback:
    force: yes
    auto: yes
    separator: ', '
replaygain:
    backend: bs1770gain
    peak: sample
    r128: Opus FLAC
    overwrite: no
    auto: yes
    targetlevel: 89
    per_disc: no
    chunk_at: 5000
    method: replaygain
zero:
    fields: images rg_track_gain rg_album_gain rg_track_peak rg_album_peak
    auto: yes
    keep_fields: []
    update_database: 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
ftintitle:
    auto: yes
    drop: no
    format: feat. {0}
edit:
    albumfields: album albumartist
    itemfields: track title artist album
    ignore_fields: id path
missing:
    count: no
    total: no
    album: no
fuzzy:
    prefix: '~'
    threshold: 0.7

[zsinskri]

This is already fixed in #3065 (via commit f3f75b7), as I found it while testing my master-merge conflict resolution. If that PR still has some time to wait, it should be easy to produce a separate fix as well, as only a single line of commit 31326eb has to be reverted.

I guess, I should also add some tests. I will do that later.

[sampsyo]

Got it; thanks for pointing this out!

I actually think the right way to fix this might be in MediaFile. That's where we intermediate between the internal representation of the number (a Python float object) and its storage in files' tags, according to various standards.