Skip to content
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

Add parser for reading from the song's tags or from a local .lrc/.lyric file #79

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ To install lLyrics from source you will need the package `gettext`.

#### Dependencies ####

lLyrics can be run without the need of any additional packages, but it is recommended to install the python module **"chardet"** for better handling of different encodings.
lLyrics can be run without the need of any additional packages, but it is recommended to install the python module **"chardet"** for better handling of different encodings. The mutagen module is also required to read from the song's metadata.



Expand Down
123 changes: 123 additions & 0 deletions lLyrics/LocalParser.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
# -*- coding: utf-8 -*-
Copy link

Choose a reason for hiding this comment

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

I wonder, is it still relevant in 2020, after Python-2's demise?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, it's useless. I just copied the template from another parser.

# Parses lyrics from a .lyric or .lrc file or from the tags of the song
Copy link

Choose a reason for hiding this comment

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

I don't get it why this repository has no global LICENSE file, or why GPL text is fragmented via blank lines like that, but one thing is for sure: doc comments should be separate, inside triple double quotes and come after license text.

Also, lets call it "track" instead of "song". Even audio books and podcasts have 'lyrics' in them sometimes.

Suggested change
# Parses lyrics from a .lyric or .lrc file or from the tags of the song
"""
Parses lyrics from a .lyric or .lrc file or from the tags of the track.
"""

# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 2 of the License, or
# (at your option) any later version.

# 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, see <http://www.gnu.org/licenses/>.

import urllib
import os.path as path
Copy link

Choose a reason for hiding this comment

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

FWIW there's a modern pathlib out there, but otherwise it's not really much different.

from mutagen.oggvorbis import OggVorbis
from mutagen.oggopus import OggOpus
from mutagen.flac import FLAC
from mutagen.id3 import ID3

class Parser(object):
def __init__(self, artist, title, song_path):
self.artist = artist
self.title = title
self.lyrics = ""
Copy link

Choose a reason for hiding this comment

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

Unused member self.lyrics.

self.song_path = song_path

def parse(self):
# convert the song url to a usable path
path = self.song_path
path = urllib.parse.unquote(path)
path = path.replace("file://", "")
dir = path[0:((path.rfind("/")) + 1)]
file_name = path[((path.rfind("/")) + 1):path.rfind(".")]
out = ""
if path.endswith("mp3"):
out = self.get_id3_lyrics(path)
elif path.endswith("ogg"):
out = self.get_ogg_lyrics(path)
elif path.endswith("opus"):
out = self.get_opus_lyrics(path)
elif path.endswith("flac"):
out = self.get_flac_lyrics(path)
if out == "":
file = self.check_for_file(dir, file_name)
Copy link

Choose a reason for hiding this comment

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

Name suggests it only checks, but in fact it opens a path and returns opened file. Since open calls are not guarded anyway, wouldn't it make more sense to return a string path here, and call open only once?

Copy link
Author

Choose a reason for hiding this comment

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

yeah returning open() is kinda sketchy anyway.

if file == "":
return ""
out = ""
for line in file.readlines():
out += line
file.close()
Copy link

Choose a reason for hiding this comment

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

No need to read line-by-line, and no need to manually close it with this syntax:

Suggested change
file.close()
with open(path) as file:
out = file.read()

Copy link
Author

Choose a reason for hiding this comment

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

neat! Didn't know you could do that.

self.lyrics = out
return out
else:
return out
def check_for_file(self, dir, file_name):
# This only checks for .lrc or .lyric files with the same name as the song or with the same "cleaned" name as
# the song. If you have files in any other format, please add it to this function.
Copy link

Choose a reason for hiding this comment

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

Doc comments

Suggested change
# This only checks for .lrc or .lyric files with the same name as the song or with the same "cleaned" name as
# the song. If you have files in any other format, please add it to this function.
"""
This only checks for .lrc or .lyric files with the same name as the song or with the same "cleaned" name as
the song. If you have files in any other format, please add it to this function.
"""

if path.isfile(dir + file_name + ".lrc"):
return open(dir + file_name + ".lrc")
elif path.isfile(dir + file_name + ".lyric"):
return open(dir + file_name + ".lyric")
elif path.isfile(dir + self.title + ".lrc"):
return open(dir + self.title + ".lrc")
elif path.isfile(dir + self.title + ".lyric"):
return open(dir + self.title + ".lyric")
else:
return ""
Copy link

@ratijas ratijas Jul 29, 2020

Choose a reason for hiding this comment

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

So, this function returns either opened file object or a string. Sounds a little bit off. Better return None as usual, and check for it via is operator. Return type will be Optional[file] (well, kind of — since the open's real type would be _io._IOBase or its derivative).

def get_id3_lyrics(self, path):
try:
file = ID3(path)
except:
return ""
if len(file.getall("USLT")) > 0:
out = file.getall("USLT")[0]
return out.text
else:
print("No lyrics!")
return ""
def get_opus_lyrics(self, path):
file = OggOpus(path)
out = ""
try:
out = file["LYRICS"]
except:
pass
try:
out = file["UNSYNCEDLYRICS"]
except:
pass
Copy link

Choose a reason for hiding this comment

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

This is weird. In case both indexing operations succeed, second would just overwrite the variable. If they are mutually exclusive, could you explicitly mark them as such, please?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, you are right

if out != "":
return out[0]
return ""
def get_ogg_lyrics(self, path):
file = OggVorbis(path)
out = ""
try:
out = file["LYRICS"]
except:
pass
try:
out = file["UNSYNCEDLYRICS"]
except:
pass
Copy link

Choose a reason for hiding this comment

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

same.

if out != "":
return out[0]
return ""
def get_flac_lyrics(self, path):
file = FLAC(path)
out = ""
try:
out = file["LYRICS"]
except:
pass
try:
out = file["UNSYNCEDLYRICS"]
except:
pass
Copy link

Choose a reason for hiding this comment

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

same.

You know, at this point, the whole file[...] retrieval deserves it's own function.

if out != "":
return out[0]
return ""
14 changes: 10 additions & 4 deletions lLyrics/lLyrics.py
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from gi.repository import GdkPixbuf
from gi.repository import GLib

import LocalParser
import ChartlyricsParser
import LyricwikiParser
import MetrolyricsParser
Expand Down Expand Up @@ -94,7 +95,7 @@
LYRICS_ARTIST_REPLACE = [("/", "-"), (" & ", " and ")]

LYRICS_SOURCES = ["Lyricwiki.org", "Letras.terra.com.br", "Metrolyrics.com", "AZLyrics.com", "Lyricsmania.com",
"Vagalume.com.br", "Genius.com", "Darklyrics.com", "Chartlyrics.com"]
"Vagalume.com.br", "Genius.com", "Darklyrics.com", "Chartlyrics.com", "Local File"]
Copy link

Choose a reason for hiding this comment

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

Is there any i18n opportunity for plugins?



class lLyrics(GObject.Object, Peas.Activatable):
Expand All @@ -117,7 +118,7 @@ def do_activate(self):
"Metrolyrics.com": MetrolyricsParser, "AZLyrics.com": AZLyricsParser,
"Lyricsmania.com": LyricsmaniaParser, "Chartlyrics.com": ChartlyricsParser,
"Darklyrics.com": DarklyricsParser, "Genius.com": GeniusParser,
"Vagalume.com.br": VagalumeParser})
"Vagalume.com.br": VagalumeParser, "Local File": LocalParser})
self.add_builtin_lyrics_sources()

# Get the user preferences
Expand Down Expand Up @@ -198,6 +199,7 @@ def do_deactivate(self):
self.lyrics_before_edit = None
self.edit_event = None
self.path_before_edit = None
self.song_url = None
self.sources = None
self.cache = None
self.lyrics_folder = None
Expand Down Expand Up @@ -472,6 +474,7 @@ def search_lyrics(self, player, entry):
# get the song data
self.artist = entry.get_string(RB.RhythmDBPropType.ARTIST)
self.title = entry.get_string(RB.RhythmDBPropType.TITLE)
self.song_url = entry.get_string(RB.RhythmDBPropType.LOCATION)

print("search lyrics for " + self.artist + " - " + self.title)

Expand Down Expand Up @@ -812,8 +815,11 @@ def get_lyrics_from_source(self, source, artist, title):

print("source: " + source)
self.current_source = source

parser = self.dict[source].Parser(artist, title)
if source == "Local File":
print(title)
parser = self.dict[source].Parser(artist, title, self.song_url)
else:
parser = self.dict[source].Parser(artist, title)
try:
lyrics = parser.parse()
except Exception as e:
Expand Down