Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10559 - lrcplayer: a plugin to view .lrc file

Attached to Project: Rockbox
Opened by Teruaki Kawashima (teru) - Sunday, 23 August 2009, 17:13 GMT+2
Last edited by Teruaki Kawashima (teru) - Saturday, 05 June 2010, 14:08 GMT+2
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This is previously posted to FS#7432.

This plugin displays lyrics in .lrc file synchronized with song being played.
See attached lrcplayer.txt for details.

chages from lrcplayer.3.patch (in FS#7432).
* disable reading id3 for low mem targets.
* ignore word time tags in enhanced .lrc format instead of displaying them.
   lrcplayer.4.patch (79.8 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2519 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2524 insertions(+)

   lrcplayer.txt (3.3 KiB)
This task depends upon

Closed by  Teruaki Kawashima (teru)
Saturday, 05 June 2010, 14:08 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  committed as r26574
Comment by Teruaki Kawashima (teru) - Thursday, 27 August 2009, 15:53 GMT+2
this is very primitive/limited version of lrcplayer only shows enhanced .lrc format.
   enhanced_lrcplayer.c (19.2 KiB)
Comment by Nick G (gersto) - Friday, 28 August 2009, 00:39 GMT+2
So far i've been unsuccessful in seeing lyrics off the ID3 tag on my MP3s. Is there a specific format/version of ID3 V2 the tag needs to be? I tag my MP3s using EvilLyrics Eviltagger.
Comment by Nick G (gersto) - Friday, 28 August 2009, 01:29 GMT+2
ok nevermind i switched from EvilLyrics to MiniLyrics and checked only the option to embed Unsynced Lyrics. EvilLyrics only uses Lyrics3 tag. Now i can see the lyrics :)
Comment by Teruaki Kawashima (teru) - Thursday, 03 September 2009, 15:22 GMT+2
Update.
* implement displaying of enhanced .lrc format.
   lrcplayer.5.patch (85.6 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2772 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2777 insertions(+)

Comment by Teruaki Kawashima (teru) - Saturday, 05 September 2009, 13:33 GMT+2
*some minor fixes and changes.
*update lrcplayer.txt.
   lrcplayer.6.patch (86.7 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2807 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2812 insertions(+)

   lrcplayer.txt (3.2 KiB)
Comment by Ralph Soto (agalloch) - Saturday, 19 September 2009, 03:22 GMT+2
Wow, compiled this a couple of weeks ago and im loving it! Nice and basic with just the right amount of features.
Only one problem though. I use minilyrics to download the .lrc files and many of them say "no lyrics" when i try to open them with lrcplayer.
Aah, I see. The files not working are encoded in utf-16, sncrlrc would convert every file into utf8, thats why i never had a problem with the files.

If you could implement the utf-8 converter it would make this program perfect.
Comment by Roman Artyuhin (bahus) - Wednesday, 14 October 2009, 13:47 GMT+2
Add support for Unicode .lrc files.
   lrcplayer.7.patch (87.8 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/lrcplayer.c    | 2842 ++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 4 files changed, 2847 insertions(+)

Comment by Teruaki Kawashima (teru) - Thursday, 15 October 2009, 11:46 GMT+2
Update patch.
   lrcplayer.8.patch (86.9 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2826 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2831 insertions(+)

Comment by Boris Nazaroff (bor_ka) - Monday, 19 October 2009, 19:49 GMT+2
Thanks for the plugin. It rocks ;)

But I think that the scheme for lrc files is not perfect, for me at least. I added a lookup in the subfolder of the current playing file. Subfolder name is taken from the settings. Attached is the diff against 8-patch.

As for lrc-file placing scheme I see 3 options.
- The first is to store it alongside the file (it works, but not very clean for me).
- The second is to keep these files in the subfolder (as in this patch).
- And the third - is to store all lyrics in the special root folder, but in this case the lrc file should be named not after mp3 file (it can be renamed or like), but %artist% - %album% - %title%.lrc. The %title% is not enough since there may be different songs with the same title (or even different versions of the same song in different albums). These lrc files could be placed in subfolder, or just en masse in the special folder.

The third one may be the most elegant, but the second is enough to me. And I don't know rockbox internals also :)
   lrcplayer.8a.diff (0.8 KiB) 
 lrcplayer.c |   28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

Comment by Boris Nazaroff (bor_ka) - Sunday, 13 December 2009, 11:43 GMT+2
Under 23943 revision the code is not working due to the changed viewscreen interface (namely, the "viewportmanager_set_statusbar" function is deprecated). The next patch contains

1) Commented out viewportmanager_set_statusbar calls - it appears that the plugin works Ok without it with the new statusbar code
2) Disabling theme backdrop on the plugin start (colourful backdrop causes difficulties in reading the lyrics)
3) The changes from my previous diff

   lrcplayer.9.patch (87.6 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2856 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2861 insertions(+)

Comment by Teruaki Kawashima (teru) - Sunday, 13 December 2009, 14:17 GMT+2
Update and resync patch.
* replace viewportmanager_set_statusbar with viewportmanager_theme_enable/viewportmanager_theme_undo and comment out them as they are not in plugin api.
* add checks for lyrics files in subfolder of current audio file (suggested by Boris Nazaroff) and remove some checks.
* use plugin buffer.
   lrcplayer.10.patch (87.9 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2869 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2874 insertions(+)

Comment by Boris Nazaroff (bor_ka) - Sunday, 13 December 2009, 16:41 GMT+2
Thanks. But your patch does leave out the theme backdrop. I think that until the "theme_enable" and "theme_undo" are made "public", one should manually disable background picture.

Try for example the AR-150 theme.
Comment by Teruaki Kawashima (teru) - Friday, 18 December 2009, 18:00 GMT+2
I prefer to display backdrop and am not going to remove it.
If the backdrop causes problem, add code to remove it or add theme_enable/theme_undo to plugin_api and uncomment relevant line by yourself.
Comment by Boris Nazaroff (bor_ka) - Saturday, 19 December 2009, 20:21 GMT+2
Yes, it is possible to manually edit a patch, but not user friendly at all IMO. The best solution would be adding a preference option.
Comment by Teruaki Kawashima (teru) - Sunday, 20 December 2009, 04:19 GMT+2
I don't think so.
   lrcplayer.11.patch (88.2 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2879 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2884 insertions(+)

Comment by braulio (alturigo) - Tuesday, 29 December 2009, 19:10 GMT+2
I've been trying to make this work on a Sansa Fuze but I couldn't. No lyrics are displayed but I can use the menu and even the Timetag Editor (so I know the correct lrc file is loaded).

Revision is 24120.
Comment by Fred Bauer (freddyb) - Wednesday, 06 January 2010, 23:33 GMT+2
I'm having the same problem as braulio on my Fuze. There is a problem with the viewport because it's trying to use vp_lyrics[i].width but it's zero. I manually set the width and height so that putsxy was getting good coordinates on the screen but it doesn't draw the text.
Comment by Teruaki Kawashima (teru) - Thursday, 07 January 2010, 04:53 GMT+2
Update patch.
   lrcplayer.12.patch (88 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2880 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2885 insertions(+)

Comment by Fred Bauer (freddyb) - Thursday, 07 January 2010, 07:20 GMT+2
v12 works well on Fuze. Thanks, it's a nice app.
Comment by Teruaki Kawashima (teru) - Saturday, 13 February 2010, 05:08 GMT+2
update patch.
* reorganized menu tree.
   lrcplayer.13.patch (89.3 KiB) 
 apps/plugins/CATEGORIES     |    1 
 apps/plugins/SOURCES        |    1 
 apps/plugins/viewers.config |    3 
 apps/plugins/lrcplayer.c    | 2940 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 2945 insertions(+)

   lrcplayer.txt (3.5 KiB)
Comment by Teruaki Kawashima (teru) - Monday, 17 May 2010, 15:01 GMT+2
update patch:
* fix bug that .lrc file may be messed messed after edited under certain condition.
*add manual.
   lrcplayer.14.patch (95.5 KiB) 
 apps/plugins/CATEGORIES      |    1 
 apps/plugins/SOURCES         |    1 
 apps/plugins/viewers.config  |    3 
 apps/plugins/lrcplayer.c     | 2945 +++++++++++++++++++++++++++++++++++++++++++
 manual/plugins/main.tex      |    2 
 manual/plugins/lrcplayer.tex |  172 ++
 6 files changed, 3124 insertions(+)

Comment by braulio (alturigo) - Monday, 17 May 2010, 17:17 GMT+2
It doesn't compile against 3.5.1 (stable).

/rockbox-v3_5_1/apps/plugins/lrcplayer.c: In function ‘load_lrc_file’:
/rockbox-v3_5_1/apps/plugins/lrcplayer.c:1026: error: too many arguments to function ‘rb->creat’
/rockbox-v3_5_1/apps/plugins/lrcplayer.c: In function ‘save_changes’:
/rockbox-v3_5_1/apps/plugins/lrcplayer.c:1975: error: too many arguments to function ‘rb->creat’
make: *** [/rockbox-v3_5_1/build/apps/plugins/lrcplayer.o] Error 1

BTW, thanks for the wonderful plugin!
Comment by Teruaki Kawashima (teru) - Monday, 17 May 2010, 17:53 GMT+2
to make it compile against 3.5.1, i think you need to remove ", 0666" in creat() in the lines shown in the error message (1066 and 1975).
Comment by braulio (alturigo) - Tuesday, 18 May 2010, 13:03 GMT+2
Last post was an easy fix :) Now, I've found that some lyrics won't be displayed if the minutes are only one number long. For example:
[0:14.612], [1:47.121] and [5:37.809] won't be displayed unless a zero is added to the minute counter like this [00:14.612], [01:47.121] and [05:37.809].
Comment by Teruaki Kawashima (teru) - Tuesday, 18 May 2010, 16:02 GMT+2
[00:14.612] still will not be displayed. it needs to be [00:14.61]. lrcplayer only recognize time tags in the formats described in manual.
if you want it to recognize tag like [0:14.612], i can add it but maybe take some time.
Comment by Michael Chicoine (mc2739) - Tuesday, 18 May 2010, 16:14 GMT+2
@ braulio

I have not seen any indication that single digits or more than two digits is an acceptable format for the time specification in an lrc file. All references I have found show the format to be [mm:ss.xx]
Comment by braulio (alturigo) - Tuesday, 18 May 2010, 16:20 GMT+2
I should have RTFM... Anyway 99% of my files have the correct format, the wrong ones come from the same guy so not a big problem. Thanks for the quick reply.

Loading...