Skip to content

add plugin to set website from relations #341

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: 2.0
Choose a base branch
from
Open

add plugin to set website from relations #341

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
291c6d2
add plugin to set website from relations
Nov 3, 2022
a2826c4
remove unused import
Nov 3, 2022
552854f
lower case type of ident, more compatible
Nov 4, 2022
1831391
correction comment
Nov 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 141 additions & 0 deletions plugins/relations_website/relations_website.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
# -*- coding: utf-8 -*-
# Musicbrainz plugin for Picard
# Copyright (C) 2022 Tobias Sarner
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License version 2 as
# published by the Free Software Foundation.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License along
# with this program; if not, write to the Free Software Foundation, Inc.,
# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
#

PLUGIN_NAME = 'Musicbrainz Relations Website'
PLUGIN_AUTHOR = 'Tobias Sarner'
PLUGIN_DESCRIPTION = '''Uses Musicbrainz relations to set website advanced.
-Album Artist Website- plugin, can used separate to set album artist(s) Official Homepage(s) and will not attached by this plugin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am entirely unclear from this description what the relationship of this plugin is to my Album Artist Website plugin. But I wonder whether this functionality would be better served by adding it to the Album Artist Website plugin (renaming it Websites Metadata perhaps) rather than creating a new one.


WARNING: Experimental plugin. All guarantees voided by use.

Example: Taylor Swift
website:artist_1_official homepage https://www.taylorswift.com/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I am not sure that you can just suffix the existing website tag with :subtag and not screw up the existing usage of website without a suffix. From memory (which is old and may be wrong) Picard knows which tags are multi-value, and AFAIK website is not one of them.
  2. website is explicitly mapped to e.g. the ID3v2 WOAR tag which is explicitly for the Album Artist official webpage. The WOAS tag is defined in ID3v2 as holding the official web page for the Release, and Picard needs a definition of a metadata name for this and a mapping to ID3 and other tagging formats to defined how it is loaded and saved.
  3. Similarly there is a WXXX:subtag tag in ID3v2 to hold miscellaneous URLs that do not have explicit tags - so e.g. the Discord URL should be saved in WXXX:discord, and again a mapping from metadata to WXXX would be needed in Picard to support this. I don't have any issues with the idea of storing the Discord URL for use in players, but if we are going to support Discord we should probably also have a Discord-Release metadata variable which holds the discord release number and Discord-Release Group to store the Discord master release number. Discord URLs can then be generated from these if you double click on them in Picard.

So, what I am saying is that ideally there would need to be a Picard enhancement to support the metadata you want to hold and make sure it gets saved in the write tags for the various formats. If someone is going to do this, then a comprehensive review of ID3 tags and other formats and a full mapping of these to metadata names would be a great enhancement IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without further knowledge Picard would just save a TXXX:website:subtag, so that would not be a problem in general. Even though some tags use colons for special behavior (e.g. comment:description), there is nothing special per se for this notation.

But anyway, I totally agree with the points you make above. It would be better if support for WXXX (and in general more of the W*** webpage tags) would exist. As a first step having website:subtag mapped to WXXX:subtag sounds sensible. But we could also directly support let's say website:pay for WPAY (PICARD-1394) or so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added PICARD-2587 as a start. Not yet sure if my suggestion is the best idea.

website:release_1_purchase https://taylorswift.universal-music.de/p51-i0602445790098/taylor-swift/midnights/index.html
website:release-group_1_https://www.discogs.com/master/2831825'''
PLUGIN_LICENSE = "GPL-2.0"
PLUGIN_LICENSE_URL = "https://www.gnu.org/licenses/gpl-2.0.txt"
PLUGIN_VERSION = "0.0.1"
PLUGIN_API_VERSIONS = ["2.0"]

from functools import partial
from picard import config, log
from picard.metadata import register_album_metadata_processor
from picard.webservice import ratecontrol
from picard.util import load_json

MUSICBRAINZ_HOST = config.setting["server_host"]
MUSICBRAINZ_PORT = config.setting["server_port"]

ratecontrol.set_minimum_delay((MUSICBRAINZ_HOST, MUSICBRAINZ_PORT), 50)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO you absolutely should not override the ratecontrol in a plugin like this.



def result_albumartist(album, metadata, data, reply, error):
prefix = "album-artist"
musicbrainz_id = metadata["musicbrainz_albumartistid"]
if error:
album._requests -= 1
album._finalize_loading(None)
return

try:
data = load_json(data).get("relations")
if data:
idx = 0
for relations in data:
if "url" in relations and "resource" in relations["url"]:
idx += 1
ident = "website:" + prefix + "_" + str(idx) + "_" + relations["type"].lower()
if ident in metadata:
metadata[ident] = review_text
else:
metadata.add(ident, relations["url"]["resource"])

log.debug("%s: %s %s (%s) Parsed response", PLUGIN_NAME, prefix, musicbrainz_id, metadata["albumtitle"])
except Exception as e:
log.error("%s: %s %s (%s) Error parsing response: %s", PLUGIN_NAME, prefix, musicbrainz_id, metadata["albumtitle"], str(e))
finally:
album._requests -= 1
album._finalize_loading(None)

def result_releasegroup(album, metadata, data, reply, error):
prefix = "release-group"
musicbrainz_id = metadata["musicbrainz_releasegroupid"]
if error:
album._requests -= 1
album._finalize_loading(None)
return

try:
data = load_json(data).get("relations")
if data:
idx = 0
for relations in data:
if "url" in relations and "resource" in relations["url"]:
idx += 1
ident = "website:" + prefix + "_" + str(idx) + "_" + relations["type"].lower()
if ident in metadata:
metadata[ident] = review_text
else:
metadata.add(ident, relations["url"]["resource"])

log.debug("%s: %s %s (%s) Parsed response", PLUGIN_NAME, prefix, musicbrainz_id, metadata["albumtitle"])
except Exception as e:
log.error("%s: %s %s (%s) Error parsing response: %s", PLUGIN_NAME, prefix, musicbrainz_id, metadata["albumtitle"], str(e))
finally:
album._requests -= 1
album._finalize_loading(None)

def process_relations(album, metadata, release):
queryargs = {
"fmt": "json",
"inc": "url-rels"
}
album.tagger.webservice.get(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I wrote the Artist website plugin, the artist official website was not available in the standard release metadata (even with the extra relations enabled) so I wrote the plugin to make an additional webservice call. BUT, since it was likely that a user might be tagging multiple albums for the same artist, I spent a lot of time crafting a caching mechanism, so that you only made 1 call per artist for the lifetime of the Picard execution process. If you are going to duplicate the functionality to make such a webservice call, you probably ought to reuse the caching code I wrote.

MUSICBRAINZ_HOST,
MUSICBRAINZ_PORT,
"/ws/2/artist/" + metadata["musicbrainz_albumartistid"],
partial(result_albumartist, album, metadata),
priority=True,
parse_response_type=None,
queryargs=queryargs
)
album._requests += 1
album.tagger.webservice.get(
MUSICBRAINZ_HOST,
MUSICBRAINZ_PORT,
"/ws/2/release-group/" + metadata["musicbrainz_releasegroupid"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Picard has also already downloaded this information - why are you repeating the call?

partial(result_releasegroup, album, metadata),
priority=True,
parse_response_type=None,
queryargs=queryargs
)
album._requests += 1
idx = 0
prefix = "release"
for relation in release["relations"]:
if "url" in relation and "resource" in relation["url"]:
idx += 1
ident = "website:" + prefix + "_" + str(idx) + "_" + relation["type"].lower()
if ident in metadata:
metadata[ident] = relation["url"]["resource"]
else:
metadata.add(ident, relation["url"]["resource"])


register_album_metadata_processor(process_relations)