Discussion:
cmus lyrics view
Max Klinger
2013-07-16 23:12:58 UTC
Permalink
Hello fine folks,

TLDR: I wrote a cmus lyrics view

I love cmus but one feature kept driving me back to mpd from time to
time: lyrics. Finally I had some quiet time in the hospital and
actually got myself to write a lyrics view.

After reading all the ranting and whining on the mailing list on this
apparently controversial topic a year ago and earlier, I tried to take
the main points into account. Still I am certain that this will spark
some discussion, that's why I did not sent a pull request first.

If you want to take a look at it now clone
http://github.com/gitkaste/cmus
The code was a bit trial and error and the commits reflect this to a
degree. Once I get the green lights here I may make a more orderly
(set of) patch(es) out of this.

The code generates a lyrics view on 7 and pushes the settings/keys to
8, my personal preference would be 5 for the lyrics though.
The view selection makes no sense to me btw. Why do you hard code the
view numbers in messages but allow the user to change them?

How to use
* set lyrics_cmd=glyrc lyrics -a $a -t $t -n 1 -w stdout -p 15 -m 8
--cache ~/.cmus/
or something similar. $a is expanded to the artist, $t title, $b album
$f filename (for generality). I couldn't think of any other
interesting tag to recognize but it's easy to implement if someone
wants more/others.
* select a song in 1-4 and run lyrics-fetch


This starts a worker job that first tries to find the lyrics tag in
the track_info and if it doesn't then it spawns off another process
that runs whatever you have in your lyrics_cmd. This is a compromise
in as far, as the lyrics view is completely content agnostic. It's a
trivial exercise to copy the cmd_lyrics_fetch and
do_update_lyrics_job, leave out the check for the lyrics tag and you
have a generic scriptable view, that can display whatever output the
non interactive cmd of your choice gives you. (I'd love to see someone
implement a cover fetch followed by libcaca)

It has the strong disadvantage that the code for grabbing the output
is a bit convoluted and it would be faster, more responsive and
cleaner to link against glyr directly instead of calling glyrc as I
did. Maybe someone wants to make this a configure option. On the other
hand the upside is that you can use any lyrics script you like. The
choice for glyr was made because it does all the heavy lifting
including caching. It doesn't support a few of the lyrics dbs I'd like
so maybe I'll patch that next time I need to procrastinate.

Upon careful review you will find several subtle changes in places you
don't expect and may find debatable, like the new worker type, the
exposed prev_view and the newlines in uchar. I can discuss these if
someone cares or objects. The rest of the code should be pretty
straightforward. Improvements very welcome.


What's not done yet:
* Direct linkage against glyr
* Construct lyrics from SYLT instead of USLT if the latter is missing.
I have never seen these in the wild, so I didn't care to do that yet.
* Any tag system other than id3 is unsupported atm. Mainly because I
only have mp3s and aacs.
* The results aren't too golden yet, mainly because minor spelling
differences ruin the search :/ If someone has a good idea about this
I'd love to hear from you.
* It'd be nice to have another command that fetches the lyrics for the
song currently playing
* It's not tested a lot yet and parts of the cmus internals are still
somewhat confusing to me. Quite likely there are a couple of stupid
bugs there, that I don't see on my system.
* Documentation isn't updated to reflect the view change.

Yours Max
Gregory Petrosyan
2013-07-18 14:10:03 UTC
Permalink
Post by Max Klinger
Hello fine folks,
TLDR: I wrote a cmus lyrics view
Wow! Very nice to see new features being implemented.

We should come up with a more generic name for it — "data" view,
maybe? I'll use this option here.
Post by Max Klinger
How to use
* set lyrics_cmd=glyrc lyrics -a $a -t $t -n 1 -w stdout -p 15 -m 8
--cache ~/.cmus/
or something similar. $a is expanded to the artist, $t title, $b album
$f filename (for generality). I couldn't think of any other
interesting tag to recognize but it's easy to implement if someone
wants more/others.
* select a song in 1-4 and run lyrics-fetch
This starts a worker job that first tries to find the lyrics tag in
the track_info and if it doesn't then it spawns off another process
that runs whatever you have in your lyrics_cmd. This is a compromise
in as far, as the lyrics view is completely content agnostic. It's a
trivial exercise to copy the cmd_lyrics_fetch and
do_update_lyrics_job, leave out the check for the lyrics tag and you
have a generic scriptable view, that can display whatever output the
non interactive cmd of your choice gives you. (I'd love to see someone
implement a cover fetch followed by libcaca)
I think that there is a way to make this both more generic and simpler to
implement. My idea is instead of `fetch.c` logic, a command could be exposed
throught `cmus-remote` that replaces the content of "data" view. With this in
place, we can use `run` or `shell` to call a script, which:

- gets current song metadata via `cmus-remote -Q`
- checks it for embedded lyrics
- if there is none, runs `glyrc`
- after `glyrc` has finished, calls `cmus-remote` to update the content of data
view

Such a scipt can be included in the cmus distribution. Additionally, people may
write their own (which e.g. implement a cover fetch followed by libcaca).
Post by Max Klinger
It has the strong disadvantage that the code for grabbing the output
is a bit convoluted and it would be faster, more responsive and
cleaner to link against glyr directly instead of calling glyrc as I
did. Maybe someone wants to make this a configure option.
When we are talking about looking up information in the internet, I think
approach of using external program is certainly better — much safer and
definitely not slower (unless we are writing a web browser :-)

Anyway — thanks a lot for your work! I strongly prefer the approach I have
outlined. What do you think about it?

Gregory

Loading...