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

Improve lyrics-in-tags support (globally and for ID3 tags in particular) #1810

Merged
merged 4 commits into from Feb 14, 2016

Conversation

Projects
None yet
2 participants
@intelfx
Contributor

intelfx commented Feb 7, 2016

  • add support for filling lyrics tag from USLT frame of ID3v2 tags
    (only empty desc and empty lang — others are ignored but not deleted)
  • synthesize tag ~lyrics by looking both at the tags and the external file (in this order)
  • in the lyrics pane and the lyrics editor, use tag ~lyrics for reading
  • in the lyrics editor, write both to the lyrics tag and to the file

This should fix #919.

@intelfx

This comment has been minimized.

Contributor

intelfx commented Feb 7, 2016

It would be good if someone advised me how to do "detection" in the lyrics editor to save lyrics to file only if saving to tags is unsupported for this file.

@@ -357,6 +363,12 @@ def write(self):
tag.add(mutagen.id3.COMM(encoding=enc, text=t, desc=u"",
lang="\x00\x00\x00"))
if "lyrics" in self:

This comment has been minimized.

@lazka

lazka Feb 7, 2016

Member

Please add a test for reading/writing those frames in test_formats__id3.py

This comment has been minimized.

@intelfx

intelfx Feb 8, 2016

Contributor

Done.

text = buffer.get_text(start, end, True)
# First, write back to the tags.
song["lyrics"] = text

This comment has been minimized.

@lazka

lazka Feb 7, 2016

Member

text isn't unicode here I think, .decode("utf-8")

This comment has been minimized.

@intelfx

intelfx Feb 8, 2016

Contributor

Done. So you mean that the text in the buffer is in the system's global encoding which is not necessarily utf-8?

If yes, then we also need to convert from utf-8 to system encoding on load — how to do that? (/me is not familiar with Python at all, this is probably my first python patch ever)

song["lyrics"] = text
try:
song.write()
except NotImplementedError:

This comment has been minimized.

@lazka

lazka Feb 7, 2016

Member

Why the NotImplementedError?

except NotImplementedError:
pass
# Then, write to file.

This comment has been minimized.

@lazka

lazka Feb 7, 2016

Member

What's this about?

This comment has been minimized.

@intelfx

intelfx Feb 8, 2016

Contributor

Hm? We write the modified text to file's tags, then write the same text to the external file (previous behavior).

BTW, it would be good if you told me how to detect whether the format handler really writes lyrics to tags (and does not ignore them, as it was with ID3 before this patch). If it really does, then we could skip writing to the file altogether. Or keep it that way for forward-compatibility with earlier Quod Libet versions?

def __delete(self, delete, lyricname, save):
def __delete(self, delete, song, save):
# First, delete from the tags.
song["lyrics"] = ""

This comment has been minimized.

@lazka

lazka Feb 7, 2016

Member

song.remove("lyrics") or song.pop("lyrics", None)

This comment has been minimized.

@intelfx

intelfx Feb 8, 2016

Contributor

Done.

song["lyrics"] = ""
try:
song.write()
except NotImplementedError:

This comment has been minimized.

@lazka

lazka Feb 7, 2016

Member

Why the NotImplementedError?

This comment has been minimized.

@intelfx

intelfx Feb 8, 2016

Contributor

The default stub implementation of write() in class AudioFile throws that.

@intelfx

This comment has been minimized.

Contributor

intelfx commented Feb 12, 2016

@lazka: ping?

lazka added a commit that referenced this pull request Feb 14, 2016

Merge pull request #1810 from intelfx/id3-lyrics
Improve lyrics-in-tags support (globally and for ID3 tags in particular)

@lazka lazka merged commit 6b23033 into quodlibet:master Feb 14, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment