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

ReplayGain plugin: manage existing RG tags better #1471

Closed
lazka opened this Issue Mar 15, 2015 · 10 comments

Comments

Projects
None yet
1 participant
@lazka
Member

lazka commented Mar 15, 2015

Original issue 1471 created by joschuaga@yahoo.de on 2014-09-26T10:46:00.000Z:

- What did you try to do?

I used the ReplayGain plugin on a folder that already had it's ReplayGain tag set (actually by the plugin itself a minute or so before).

- What did you expect to happen?

I expected the plugin to skip (in this case mp3) files that already had it set (or that there would be a checkbox to rescan files).

- What did happen instead?

As the progress bar indicated it rescanned the files. (I want to add ReplayGain to my whole mp3 / flac / ogg library and this would increase the amount of time immensely.)

- Which version of Quod Libet?

3.2.1

- Which operating system including version (Ubuntu 14.04, Win7, Debian
sid)?

Ubuntu 14.04 / 64bit

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #1 originally posted by joschuaga@yahoo.de on 2014-09-26T10:47:56.000Z:

To bad you can't edit you bug report here.. I guess this is more of an enhancement report than a defect. I wonder, what do you think about moving the code to another site like GitHub or BitBucket?

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #2 originally posted by reiter.christoph on 2014-09-26T11:07:20.000Z:

Yeah, we could add an option. Thanks for the report.

You could also use replaygain_album_gain="" in the album browser to get untagged ones in the meantime.

Re github etc: Yeah, not being able to edit is the thing I miss the most here. I guess we will switch some time. Atm I'm not annoyed enough to do the work (migrating bug reports etc.)

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #3 originally posted by nick.boultbee on 2014-09-26T12:33:37.000Z:

Agreed that it could be better for some users (though I also use an equivalent to Christoph's query, so it doesn't affect me much).

If we did it (I'll have a look as soon as I get some free time), I'd suggest a global plugin config drop-down for behaviour and keeping the default is "replace existing" (possible interesting additions: "alert on [significant] change").

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #4 originally posted by joschuaga@yahoo.de on 2014-09-26T12:47:03.000Z:

Thanks for the replaygain_album_gain="" trick! :)

I've found some information to automate migrating issues from Google Code to GitHub / BitBucket: https://answers.atlassian.com/questions/297709/migrating-issues-from-google-code-to-bitbucket

Maybe this could help. The code seems to be easy to migrate, but you probably knew that:

https://confluence.atlassian.com/display/BITBUCKET/Import+code+from+an+existing+project#Importcodefromanexistingproject-Importfromahostingsiteorprojectusingtheimporter

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #5 originally posted by nick.boultbee on 2014-10-04T14:26:09.000Z:

<empty>

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #6 originally posted by nick.boultbee on 2014-12-16T22:14:49.000Z:

Attached is a (first) patch that allows skipping over albums / songs with (some) tags already present.

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #7 originally posted by reiter.christoph on 2014-12-17T13:57:02.000Z:

Please fix "./setup.py quality"

diff --git a/quodlibet/quodlibet/ext/songsmenu/replaygain.py b/quodlibet/quodlibet/ext/songsmenu/replaygain.py
--- a/quodlibet/quodlibet/ext/songsmenu/replaygain.py
+++ b/quodlibet/quodlibet/ext/songsmenu/replaygain.py
@@ -2,7 +2,7 @@

ReplayGain Album Analysis using gstreamer rganalysis element

Copyright (C) 2005,2007,2009 Michael Urman

-# 2012 Nick Boultbee
+# 2012,14 Nick Boultbee

2013 Christoph Reiter

This program is free software; you can redistribute it and/or modify

@@ -15,6 +15,9 @@
from gi.repository import Pango
from gi.repository import Gst
from gi.repository import GLib
+from quodlibet import print_d
+from quodlibet.plugins import PluginConfigMixin
+
from quodlibet.browsers.collection.models import EMPTY

from quodlibet.qltk.views import HintedTreeView
@@ -35,11 +38,20 @@
return threads

+class UpdateMode(object):

  • """Enum-like class for update strategies"""
  • ALWAYS = "always"
  • ALBUM_MISSING = "album_tags_missing"
  • ANY_MISSING = "any_tags_missing"

class RGAlbum(object):

  • def init(self, rg_songs):

  • def init(self, rg_songs, process_mode):
    self.songs = rg_songs
    self.gain = None
    self.peak = None

  •    self.__should_process = None
    
  •    self.__process_mode = process_mode
    

    @Property
    def progress(self):
    @@ -86,8 +98,26 @@
    song._write(self.gain, self.peak)

    @classmethod

  • def from_songs(self, songs):

  •    return RGAlbum([RGSong(s) for s in songs])
    
  • def from_songs(cls, songs, process_mode=UpdateMode.ALWAYS):

  •    return RGAlbum([RGSong(s) for s in songs], process_mode)
    
  • @Property

  • def should_process(self):

  •    """Returns true if the album needs analysis, according to prefs"""
    
  •    if self.__should_process is None:
    
  •        mode = self.__process_mode
    
  •        if mode == UpdateMode.ALWAYS:
    
  •            process = True
    
  •        elif mode == UpdateMode.ANY_MISSING:
    
  •            process = not all([s.has_all_rg_tags for s in self.songs])
    
  •        elif mode == UpdateMode.ALBUM_MISSING:
    
  •            process = not all([s.album_gain for s in self.songs])
    
  •        else:
    
  •            print_w("Invalid setting for update mode: " + mode)
    
  •            # Safest to re-process probably.
    
  •            process = True
    
  •        self.__should_process = process
    
  •    return self.__should_process
    

You can use util.cached_property here if you need caching.
If you don't need caching should_process(process_mode) seems nicer (less state
in RGAlbum).

class RGSong(object):
@@ -98,20 +128,28 @@
self.peak = None
self.progress = 0.0
self.done = False

  •    # TODO: support prefs for not overwriting individual existing tags.
    
  •    self.overwrite_existing = True
    

I'm wondering why someone would not want to override existing values?

 def _write(self, album_gain, album_peak):
     if self.error or not self.done:
         return
     song = self.song
  •    if self.gain is not None:
    
  •        song['replaygain_track_gain'] = '%.2f dB' % self.gain
    
  •    if self.peak is not None:
    
  •        song['replaygain_track_peak'] = '%.4f' % self.peak
    
  •    if album_gain is not None:
    
  •        song['replaygain_album_gain'] = '%.2f dB' % album_gain
    
  •    if album_peak is not None:
    
  •        song['replaygain_album_peak'] = '%.4f' % album_peak
    
  •    def write_to_song(tag, pattern, value):
    
  •        if not value:
    
  •            return
    
  •        existing = song(tag, None)
    
  •        if existing and not self.overwrite_existing:
    
  •            print_d("Not overwriting existing tag %s (=%s) for %s"
    
  •                    % (tag, existing, self.song("~filename")))
    
  •            return
    
  •        song[tag] = pattern % value
    
  •    write_to_song('replaygain_track_gain', '%.2f dB', self.gain)
    
  •    write_to_song('replaygain_track_peak', '%.4f', self.peak)
    
  •    write_to_song('replaygain_album_gain', '%.2f dB', album_gain)
    
  •    write_to_song('replaygain_album_peak', '%.4f', album_peak)
    

    @Property
    def title(self):
    @@ -125,6 +163,27 @@
    def length(self):
    return self.song("~#length")

  • @Property

  • def track_gain(self):

  •    return self.song("~#replaygain_track_gain")
    
  • @Property

  • def album_gain(self):

  •    return self.song("~#replaygain_album_gain")
    
  • @Property

  • def track_peak(self):

  •    return self.song("~#replaygain_track_peak")
    
  • @Property

  • def album_peak(self):

  •    return self.song("~#replaygain_album_peak")
    
  • @Property

  • def has_all_rg_tags(s):

  •    return bool(s.track_gain and s.album_gain
    
  •                and s.track_peak and s.album_peak)
    

The properties return a mix of str/float and are unused afaics, please
move the logic into has_all_rg_tags and a new has_album_gain maybe.
Although unlikely, song("~#replaygain_track_gain") can return 0, so this
isn't the right check.

class ReplayGainPipeline(GObject.Object):

@@ -272,16 +331,22 @@

class RGDialog(Gtk.Dialog):

  • def init(self, albums, parent):

  • def init(self, albums, parent, process_mode):
    super(RGDialog, self).init(
    title=_('ReplayGain Analyzer'), parent=parent,
    buttons=(Gtk.STOCK_CANCEL, Gtk.ResponseType.CANCEL,
    Gtk.STOCK_SAVE, Gtk.ResponseType.OK)
    )

  •    self.set_default_size(500, 350)
    
  •    self.process_mode = process_mode
    
  •    self.set_default_size(600, 400)
     self.set_border_width(6)
    
  •    hbox = Gtk.HBox(spacing=6)
    
  •    info = Gtk.Label()
    
  •    hbox.pack_start(info, True, True, 0)
    
  •    self.vbox.pack_start(hbox, False, False, 6)
    
    • swin = Gtk.ScrolledWindow()
      swin.set_policy(Gtk.PolicyType.AUTOMATIC, Gtk.PolicyType.AUTOMATIC)
      swin.set_shadow_type(Gtk.ShadowType.IN)
      @@ -307,6 +372,7 @@
      def track_cdf(column, cell, model, iter_, *args):
      item = model[iter_][0]
      cell.set_property('text', item.title)
  •        cell.set_sensitive(model[iter_][1])
    
     column = Gtk.TreeViewColumn(_("Track"))
     column.set_expand(True)
    

    @@ -320,6 +386,7 @@
    def progress_cdf(column, cell, model, iter_, *args):
    item = model[iter_][0]
    cell.set_property('value', int(item.progress * 100))

  •        cell.set_sensitive(model[iter_][1])
    
     column = Gtk.TreeViewColumn(_("Progress"))
     column.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
    

    @@ -334,6 +401,7 @@
    cell.set_property('text', "-")
    else:
    cell.set_property('text', "%.2f db" % item.gain)

  •        cell.set_sensitive(model[iter_][1])
    
     column = Gtk.TreeViewColumn(_("Gain"))
     column.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
    

    @@ -348,6 +416,7 @@
    cell.set_property('text', "-")
    else:
    cell.set_property('text', "%.2f" % item.peak)

  •        cell.set_sensitive(model[iter_][1])
    
     column = Gtk.TreeViewColumn(_("Peak"))
     column.set_sizing(Gtk.TreeViewColumnSizing.AUTOSIZE)
    

    @@ -356,32 +425,39 @@
    column.set_cell_data_func(peak_renderer, peak_cdf)
    view.append_column(column)

  •    # create as many pipelines as threads
    
  •    self.pipes = []
    
  •    for i in xrange(get_num_threads()):
    

- self.pipes.append(ReplayGainPipeline())

  •    self.create_pipelines()
     self._timeout = None
     self._sigs = {}
     self._done = []
    
  •    self._todo = list([RGAlbum.from_songs(a) for a in albums])
    
  •    self.__fill_view(view, albums)
    
  •    num_to_process = sum(int(rga.should_process) for rga in self._todo)
    
  •    template = ngettext("There is <b>%d</b> album to update (of %d)",
    
  •                        "There are <b>%d</b> albums to update (of %d)",
    
  •                        num_to_process)
    
  •    info.set_markup(template % (num_to_process, len(self._todo)))
    
  •    self.connect("destroy", self.__destroy)
    
  •    self.connect('response', self.__response)
    
  • def create_pipelines(self):
  •    # create as many pipelines as threads
    
  •    self.pipes = [ReplayGainPipeline() for _ in xrange(get_num_threads())]
    
  • def __fill_view(self, view, albums):
  •    self._todo = [RGAlbum.from_songs(a, self.process_mode) for a in albums]
     self._count = len(self._todo)
    
  •    # fill the view
    
  •    self.model = model = Gtk.TreeStore(object)
    
  •    self.model = model = Gtk.TreeStore(object, bool)
     insert = model.insert
     for album in reversed(self._todo):
    
  •        base = insert(None, 0, row=[album])
    
  •        enabled = album.should_process
    
  •        base = insert(None, 0, row=[album, enabled])
         for song in reversed(album.songs):
    
  •            insert(base, 0, row=[song])
    
  •            insert(base, 0, row=[song, enabled])
     view.set_model(model)
    
     if len(self._todo) == 1:
         view.expand_all()
    
  •    self.connect("destroy", self.__destroy)
    

- self.connect('response', self.__response)

 def start_analysis(self):
     self._timeout = GLib.idle_add(self.__request_update)

@@ -393,7 +469,24 @@
p.connect("done", self.__done),
p.connect("update", self.__update),
]

  •        p.start(self._todo.pop(0))
    
  •        album = self.get_next_album()
    
  •        if not album:
    
  •            return
    
  •        p.start(album)
    
  • def get_next_album(self):

  •    next_album = None
    
  •    while not next_album:
    
  •        if not self._todo:
    
  •            print_d("No more albums to process")
    
  •            return None
    
  •        next_album = self._todo.pop(0)
    
  •        if not next_album.should_process:
    
  •            print_d("%s needs no processing" % next_album.title)
    
  •            self._done.append(next_album)
    
  •            self.__update_view_for(next_album)
    
  •            next_album = None
    
  •    return next_album
    

    def __response(self, win, response):
    if response == Gtk.ResponseType.CANCEL:
    @@ -429,7 +522,9 @@
    self._done.append(album)
    if self._todo:
    pipeline.start(self._todo.pop(0))

  •    self.__update_view_for(album)
    
  • def __update_view_for(self, album):
    for row in self.model:
    row_album = row[0]
    if row_album is album:
    @@ -447,14 +542,19 @@
    return False

-class ReplayGain(SongsMenuPlugin):
+class ReplayGain(SongsMenuPlugin, PluginConfigMixin):
PLUGIN_ID = 'ReplayGain'
PLUGIN_NAME = _('Replay Gain')

  • PLUGIN_DESC = _('Analyzes ReplayGain with gstreamer, grouped by album')
  • PLUGIN_DESC = _('Analyzes and updates ReplayGain information,'
  •                'using gstreamer. Results are grouped by album')
    
    PLUGIN_ICON = Gtk.STOCK_MEDIA_PLAY
  • A little arbitrary, but seemed to match the history best.

  • PLUGIN_VERSION = '2.1'
  • CONFIG_SECTION = 'replaygain'

def plugin_albums(self, albums):

  •    win = RGDialog(albums, parent=self.plugin_window)
    
  •    mode = self.config_get("process_if", UpdateMode.ALWAYS)
    
  •    win = RGDialog(albums, parent=self.plugin_window, process_mode=mode)
     win.show_all()
     win.start_analysis()
    

@@ -464,6 +564,67 @@
def __plugin_done(self, win):
self.plugin_finish()

  • @classmethod

  • def PluginPreferences(cls, parent):

  •    vb = Gtk.VBox(spacing=12)
    
  •    # Server settings Frame
    
  •    frame = Gtk.Frame(label=_("<b>Existing Tags</b>"))
    
  •    frame.set_shadow_type(Gtk.ShadowType.NONE)
    
  •    frame.get_label_widget().set_use_markup(True)
    
  •    frame_align = Gtk.Alignment.new(0, 0, 1, 1)
    
  •    frame_align.set_padding(6, 6, 12, 12)
    
  •    frame.add(frame_align)
    
  •    # Tabulate all settings for neatness
    
  •    table = Gtk.Table(n_rows=1, n_columns=2)
    
  •    table.set_col_spacings(6)
    
  •    table.set_row_spacings(6)
    
  •    rows = []
    
  •    def process_option_changed(combo):
    
  •        #xcode = combo.get_child().get_text()
    
  •        model = combo.get_model()
    
  •        lbl, value = model[combo.get_active()]
    
  •        cls.config_set("process_if", value)
    
  •    def create_model():
    
  •        model = Gtk.ListStore(str, str)
    
  •        model.append([_("<b>always</b>"), UpdateMode.ALWAYS])
    
  •        model.append([_("if <b>any</b> RG tags are missing"),
    
  •                      UpdateMode.ANY_MISSING])
    
  •        model.append([_("if <b>album</b> RG tags are missing"),
    
  •                      UpdateMode.ALBUM_MISSING])
    
  •        return model
    
  •    def set_active(value):
    
  •        for i, item in enumerate(model):
    
  •            if value == item[1]:
    
  •                combo.set_active(i)
    
  •    model = create_model()
    
  •    combo = Gtk.ComboBox(model=model)
    
  •    set_active(cls.config_get("process_if", UpdateMode.ALWAYS))
    
  •    renderer = Gtk.CellRendererText()
    
  •    combo.connect('changed', process_option_changed)
    
  •    combo.pack_start(renderer, True)
    
  •    combo.add_attribute(renderer, "markup", 0)
    
  •    rows.append((_("_Process albums:"), combo))
    
  •    for (row, (label_text, entry)) in enumerate(rows):
    
  •        label = Gtk.Label(label=label_text)
    
  •        label.set_alignment(0.0, 0.5)
    
  •        label.set_use_underline(True)
    
  •        label.set_mnemonic_widget(entry)
    
  •        table.attach(label, 0, 1, row, row + 1,
    
  •                     xoptions=Gtk.AttachOptions.FILL)
    
  •        table.attach(entry, 1, 2, row, row + 1)
    
  •    frame_align.add(table)
    
  •    vb.pack_start(frame, True, True, 0)
    
  •    return vb
    

    if not Gst.Registry.get().find_plugin("replaygain"):
    all = []
    diff --git a/quodlibet/tests/plugin/init.py b/quodlibet/tests/plugin/init.py
    --- a/quodlibet/tests/plugin/init.py
    +++ b/quodlibet/tests/plugin/init.py
    @@ -28,8 +28,10 @@

    make sure plugins only raise expected errors

    for name, err in ms.failures.items():

  • assert issubclass(

  •    type(err.exception), (PluginImportException, ImportError))
    
  • exc = err.exception

  • assert issubclass( type(exc), (PluginImportException, ImportError)),\

  •    "%s shouldn't have raised a %s, but it did (%r)."\
    
  •    % (name, type(exc), exc)
    

    plugins = {}
    modules = {}

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #8 originally posted by nick.boultbee on 2014-12-17T17:40:56.000Z:

Thanks again for the review.

  • Fixed the quality issues (see also revision 7a53c1c).
  • The future usage for was for light-touch usage, i.e. don't touch files at all if they don't need updating (incremental backup-friendly).
  • Use util.cached_property, thanks
  • Fixed the 0.0 bug / gotcha, thanks
  • Added more tests..
@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #9 originally posted by nick.boultbee on 2014-12-17T21:56:11.000Z:

Committed in revision 33e5892

@lazka

This comment has been minimized.

Member

lazka commented Mar 15, 2015

Comment #10 originally posted by nick.boultbee on 2014-12-23T09:24:53.000Z:

<empty>

@lazka lazka closed this Mar 15, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment