Bugfix: Tag writing with "write_rating_to_audio_file_metadata" (#863)

* Tags sometimes are not written to file metadata if "write_rating_to_audio_file_metadata" is active
* If tags are writable, rating was always written, regardless of write_rating_to_audio_file_metadata

xl/metadata/_base.py

@@ -33,7 +33,7 @@
 
 logger = logging.getLogger(__name__)
 
-from xl import version
+from xl import version, settings
 import mutagen
 
 version.register('Mutagen', mutagen.version_string)
@@ -190,7 +190,7 @@ def read_all(self):
             # __ is used to denote exaile's internal tags, so we skip
             # loading them to avoid conflicts. usually this shouldn't be
             # an issue.
-            if t.startswith("__") and t != '__rating':
+            if self._remove_internal_tag(t):
                 continue
             tags.append(t)
         alltags = self.read_tags(tags)
@@ -274,7 +274,7 @@ def write_tags(self, tagdict):
             # tags starting with __ are internal and should not be written
             # -> this covers INFO_TAGS, which also shouldn't be written
             for tag in list(tagdict.keys()):
-                if tag.startswith("__") and tag != '__rating':
+                if self._remove_internal_tag(tag):
                     del tagdict[tag]
 
             # Only modify the tags we were told to modify
@@ -353,6 +353,17 @@ def _rating_to_stars(self, data: int) -> int:
             rating = 100
         return rating
 
+    def _remove_internal_tag(self, tag):
+        if tag.startswith("__") and (
+            tag != '__rating'
+            or not settings.get_option(
+                'collection/write_rating_to_audio_file_metadata', False
+            )
+        ):
+            return True
+
+        return False
+
 
 class CaseInsensitiveBaseFormat(BaseFormat):
     case_sensitive = False

xl/metadata/_id3.py

@@ -153,7 +153,7 @@ def _set_tag(self, raw, tag, data):
             frames = [id3.WOAR(encoding=3, url=d) for d in data]
         elif tag == 'POPM':
             # Rating Stars
-            data = int(data)
+            data = int(data[0])
             rating = self._stars_to_rating(data)
             email = settings.get_option(
                 'collection/write_rating_to_audio_file_metadata_popm_mail'

xl/metadata/flac.py

@@ -81,7 +81,7 @@ def _set_tag(self, raw, tag, value):
 
         elif tag == 'rating':
             # Rating Stars
-            value = [str(self._stars_to_rating(int(value)))]
+            value = [str(self._stars_to_rating(int(value[0])))]
 
         else:
             # flac has text based attributes, so convert everything to unicode

xl/metadata/ogg.py

@@ -75,7 +75,7 @@ def _set_tag(self, raw, tag, value):
                 new_value.append(tmp)
             value = new_value
         elif tag == 'rating':
-            rating = self._stars_to_rating(value)
+            rating = self._stars_to_rating(value[0])
             value = [str(rating)]
         else:
             # vorbis has text based attributes, so convert everything to unicode

xl/trax/track.py

@@ -507,6 +507,13 @@ def _xform_set_values(self, tag, values):
             if not tag.startswith("__"):  # internal tags dont have to be lists
                 values = [values]
 
+        if (
+            tag == '__rating'
+            and not isinstance(values, list)
+            and self._write_rating_to_disk()
+        ):
+            values = [values]
+
         # For lists, filter out empty values
         if isinstance(values, list):
             values = [v for v in values if v not in (None, '')]
@@ -603,6 +610,11 @@ def get_tag_raw(self, tag, join=False):
             # TODO: This is only necessary because some places that deal with
             # __startoffset don't check for None. Those need to be fixed.
             value = self.__tags.get(tag, 0)
+        elif tag == '__rating':
+            try:
+                value = self.__tags.get(tag)[0]
+            except TypeError as e:
+                value = self.__tags.get(tag)
         else:
             value = self.__tags.get(tag)
 
@@ -923,13 +935,7 @@ def set_rating(self, rating):
         rating = 100 * rating / maximum
         self.set_tags(__rating=rating)
 
-        f = self._get_format_obj()
-        if f is None:
-            return rating
-        keys = f.tag_mapping
-        if '__rating' in keys and settings.get_option(
-            'collection/write_rating_to_audio_file_metadata', False
-        ):
+        if self._write_rating_to_disk():
             self.set_tag_disk('__rating', rating)
         return rating
 
@@ -1091,5 +1097,21 @@ def _get_track_count(cls):
         '''Internal API, returns number of track objects we have'''
         return len(cls._Track__tracksdict)
 
+    def _write_rating_to_disk(self):
+        if not settings.get_option(
+            'collection/write_rating_to_audio_file_metadata', False
+        ):
+            return False
+
+        f = self._get_format_obj()
+        if f is None:
+            return False
+
+        keys = f.tag_mapping
+        if not '__rating' in keys:
+            return False
+
+        return True
+
 
 event.add_callback(Track._the_cuts_cb, 'collection_option_set')