Rockbox

Tasklist

FS#7432 - sncviewer - a plugin for viewing synchronised lyrics on player screen

Attached to Project: Rockbox
Opened by Eddy (bascule) - Saturday, 14 July 2007, 08:59 GMT
Last edited by Eddy (bascule) - Saturday, 14 July 2007, 09:09 GMT
Task Type Patches
Category Plugins
Status New
Assigned To No-one
Operating System Another
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

This is a plugin designed to view the contents of either:
synchronised lyrics file (.snc/.lrc)
synchronised lyrics tag information (id3v2.3 SYLT - MP3 only)
unsynchronised lyrics file (.txt)
picture file containing lyrics (.bmp)

Button mappings currently coded for the following players:
iriver H1x0
iriver H3x0
iPod
iAudio X5
Toshiba Gigabeat

Attached:
sncviewer.txt - Instructions
sncviewer.patch - diff/patch file

Forum thread:
http://forums.rockbox.org/index.php?topic=2372.0
.
This task depends upon

Comment by Eddy (bascule) - Saturday, 14 July 2007, 09:01 GMT
Please note installation instructions are currently incorrect. The .patch file needs only to be applied like any other patch. There is no sncviewer.c file as stated.
Operating instuctions are fine.
Comment by x (vmh) - Saturday, 14 July 2007, 15:06 GMT
update:
- added: countdown marker
- some other small changes
Comment by Nikkhil (AceNik) - Saturday, 14 July 2007, 18:33 GMT
guys i love this patch , but can i or anyone add button codes for the iRiver H10 , will it work fine then ? or there is some other code needed too ?
Comment by Charles Philip Chan (cpchan) - Saturday, 14 July 2007, 19:26 GMT
Add button mapping for the e200 target.
Comment by x (vmh) - Saturday, 14 July 2007, 21:18 GMT
iRiver H10 button mapping added
Comment by Charles Philip Chan (cpchan) - Sunday, 15 July 2007, 02:31 GMT
Scroll wheel for volume fixed and documentation added for the e200 target.
Comment by Charles Philip Chan (cpchan) - Sunday, 15 July 2007, 02:34 GMT
Oops, something strange happened to attaching the documentation. Here it is again.
Comment by Charles Philip Chan (cpchan) - Sunday, 15 July 2007, 03:28 GMT
Oops, something strange happened to attaching the documentation. Here it is again.
Comment by x (vmh) - Sunday, 15 July 2007, 06:35 GMT
I also got some warnings. I think the special characters (start and end tag) of a snc cause the problem.
Comment by x (vmh) - Friday, 20 July 2007, 14:30 GMT
update:
- added: cue sheet support (create (auto cue)/edit/save)

this update doesn't relate to lyrics, but it's useful for music podcast.
Comment by Nikkhil (AceNik) - Friday, 20 July 2007, 22:11 GMT
guys apple has applied for a patent on this same feature for their ipods read more here

http://gizmodo.com/gadgets/apple/ipod-karaoke-patent-brings-fear-to-mass-transit-riders-280331.php

ps:can we have the highlight text thing in our sncviewer as well?
Comment by x (vmh) - Saturday, 21 July 2007, 16:33 GMT
"ps:can we have the highlight text thing in our sncviewer as well?"

If you mean highlight the word according to the word being sung, then the answer is no.
I can't find any prefabricated enhanced lrc files and I can't imagine I would do it manually.
Comment by x (vmh) - Sunday, 22 July 2007, 15:48 GMT
fixed: displaying title and artist containing characters not covered by sysfont
Comment by Kévin Ferrare (TiMiD) - Monday, 23 July 2007, 01:49 GMT
I just looked at the code of your plugin and I have some suggestions :
- For your keymappings, there are already standard actions defined in pluginlib_actions.h, maybe you should consider using some of them, that would reduce your source size
- If you want to handle multiple displays, use the "struct screen* screens[NB_SCREENS];" in the plugin API, that avoids code duplication (see other plugins)
- too much global variables makes the code difficult to follow, try to group them in structures and to pass those structures as functions parameters. That would make your code *WAY* easier to read
- externalize the bitmaps you use (see other plugins)
- split your sources into different files if possible ... +2000 lines is quite a lot
- if you are aiming for inclusion in SVN, follow the naming conventions and code guidelines of the project

Else it's a nice app to use !
Comment by x (vmh) - Monday, 23 July 2007, 05:16 GMT
- For your keymappings, there are already standard actions defined in pluginlib_actions.h, maybe you should consider using some of them, that would reduce your source size
At first glance it seems that I can use the action definition from there.

- If you want to handle multiple displays, use the "struct screen* screens[NB_SCREENS];" in the plugin API, that avoids code duplication (see other plugins)
- externalize the bitmaps you use (see other plugins)
I'll have a look at them.

- too much global variables makes the code difficult to follow, try to group them in structures and to pass those structures as functions parameters. That would make your code *WAY* easier to read
- split your sources into different files if possible ... +2000 lines is quite a lot
- if you are aiming for inclusion in SVN, follow the naming conventions and code guidelines of the project
Somehow I'm more interested in adding functionality than to (re)structure the code. But you're right it'll make it easier for others to read and maintain the code.
Comment by x (vmh) - Monday, 23 July 2007, 18:04 GMT
as suggested:
changed: use some predefined keymappings from pluginlib_actions.h
changed: use "struct screen"
Comment by x (vmh) - Monday, 06 August 2007, 18:36 GMT
update:
- adapting to the new rocks directory structure (rocks/apps)
- comments: // -> /* */
- some optimisations
Comment by x (vmh) - Thursday, 09 August 2007, 20:21 GMT
update:
- added: load translation file
- sncviewer.txt updated
Comment by x (vmh) - Thursday, 16 August 2007, 19:29 GMT
fixed: some ab-repeat issues
Comment by Shiloh Hawley (gree665) - Thursday, 23 August 2007, 17:47 GMT
I have unsynchronized lyrics in all my mp3s (in the id3v2.3 USLT tag). From reading the description, it doesn't look like I can read 'em with this patch. It would be cool if I could though, is this a possibility?
Comment by x (vmh) - Thursday, 23 August 2007, 19:12 GMT
there is already a patch which reads USLT from an id3 tag
see [url]http://www.rockbox.org/tracker/2999[/url]
Comment by Mike Miller (mikeage) - Sunday, 02 September 2007, 04:22 GMT
I just started using this patch... great work!

One note so far: for my iPod 4G, the scroll is backwards for volume. I would think that scrolling CW should _increase_ the volume, and CCW should decrease it.

Thanks!
Comment by Mike Miller (mikeage) - Sunday, 02 September 2007, 06:03 GMT
The scrolling is universally backwards on iPods (confirmed in the code).

One other note: if I start from a .txt file, and syncronize it to create a .lrc, is it supposed to delete the text file?
Comment by x (vmh) - Sunday, 02 September 2007, 08:01 GMT
volume:
oh, I see that the button definition in plugin_lib is not the same as I had defined in the code before using the lib. I assume you have the same issue with scrolling if you are in edit/browser mode?

I will fix it asap (Thursday/Friday).

text -> lrc:
yes, why do you want to keep the text file if have a lrc?
Comment by Mike Miller (mikeage) - Sunday, 02 September 2007, 08:11 GMT
re: scrolling: Correct

In theory, I don't need to, but here's the actual use case. I have a script which automatically downloads .txt lyrics for my MP3s (not working with other formats yet). If the .txt exists, it doesn't try and download a new one. Otherwise, it searches several sites and tries to find something.

I've since modified it to accept either a .lrc or a .txt file as evidence of valid lyrics, so I guess it's ok. I just didn't see anywhere in the docs that it would delete the old file.
Comment by x (vmh) - Sunday, 02 September 2007, 18:44 GMT
It's not explicitly mentioned, but line 135 in the doc "... the plugin will automatically check if the tagging is completed and it will rename the file extension to lrc/lrc8" implies it.

Your script sounds interesting. Where can I download it?
Comment by Mike Miller (mikeage) - Monday, 03 September 2007, 04:17 GMT
You're right about the document; I didn't read carefully enough

http://mikeage.net/2007/09/03/batch-downloading-of-lyrics-for-mp3/
Comment by x (vmh) - Saturday, 15 September 2007, 18:34 GMT
update:
fixed: ipod scrollbutton assignment
changed: save translation and peakmeter settings
Comment by Simon Wenger (musician72) - Saturday, 15 September 2007, 19:29 GMT
Can't compile:

CC sncviewer.c
sncviewer.c:74: error: ‘BUTTON_MODE’ undeclared here (not in a function)
sncviewer.c:75: warning: missing initializer
sncviewer.c:75: warning: (near initialization for ‘button_context_snc[11].button_code’)
make[2]: *** [/home/simon/Desktop/iaudio/bleeding/rockbox/buildsim/apps/plugins/sncviewer.o] Error 1
make[1]: *** [rocks] Error 2
make: *** [build] Fehler 2

I don't understand the error. The mapping is declared, target is an X5.
Comment by x (vmh) - Saturday, 15 September 2007, 20:15 GMT
x5 doesn't have a button called BUTTON_MODE (#endif was at the wrong place)
Comment by x (vmh) - Sunday, 16 September 2007, 16:31 GMT
update:
fixed: problems related to load translation file
fixed: replace /,\,?,: with _ on loading album art (path/<album>.bmp)
Comment by x (vmh) - Monday, 17 September 2007, 17:11 GMT
update:
- changed: capitalize first letter in menu and splash screens
- changed: switch backlight behaviour also when plugged (use functions in helper.c)
- changed: replace these chars " | < > and * ? in <album>.bmp too
Comment by Nikkhil (AceNik) - Monday, 17 September 2007, 17:19 GMT
guys what can i do with the txt file attached, does t need to be placed somewhere for making the build i never did this till now?
Comment by x (vmh) - Monday, 17 September 2007, 17:31 GMT
Do you mean the sncviewer.txt? It's only a documentation. Only the sncviewer.patch is needed for building.
Comment by x (vmh) - Tuesday, 18 September 2007, 16:44 GMT
changed: don't use the function in helper.c because it doesn't turn on the backlight if the backlight is set to 0 (off)
Comment by x (vmh) - Saturday, 22 September 2007, 12:02 GMT
fixed: don't add time offset to not initialized time tags
fixed: unify behaviour for leaving plugin (quit plugin, audio stopped, usb connected)
Comment by x (vmh) - Monday, 01 October 2007, 11:38 GMT
some fixes ...
Comment by x (vmh) - Friday, 12 October 2007, 16:41 GMT
changed:
- save temporary tagged txt files in lrc-format
- reorder some functions
Comment by Mike Miller (mikeage) - Tuesday, 16 October 2007, 14:45 GMT
If I may submit a feature request...

I download most of my lyrics automatically (see above). Often, that means I get the wrong song. It would be very nice if there was a way to delete the (wrong) lyrics file from within the plugin.

Thanks!
Comment by x (vmh) - Tuesday, 16 October 2007, 18:18 GMT
no problem, it's not a big thing to add it.

update:
added: delete lyrics entry in the menu
Comment by Mike Miller (mikeage) - Tuesday, 16 October 2007, 20:24 GMT
nice, thanks!

i'll try it out on my commute tomorrow (don't worry, I don't drive myself to work <g>)
Comment by x (vmh) - Friday, 26 October 2007, 17:24 GMT
update:
fixed: doesn't notice track change in the recent build
changed: translation icon
Comment by x (vmh) - Tuesday, 06 November 2007, 20:10 GMT
added: USLT support
changed: capitalize enums, rename some variables, ...
Comment by x (vmh) - Saturday, 17 November 2007, 09:25 GMT
changed: track change handling, some other internal functions
Comment by x (vmh) - Wednesday, 23 January 2008, 09:31 GMT
fixed: backlight
changed: simplify track change handling
Comment by x (vmh) - Thursday, 24 January 2008, 13:16 GMT
fix a bug in yesterday's version: adjusting the volume will lead to a crash.
Comment by x (vmh) - Monday, 11 February 2008, 13:13 GMT
updated: sncviewer.txt
changed: scrolling in editor
fixed: some bugs
Comment by x (vmh) - Wednesday, 13 February 2008, 11:06 GMT
changed: rename all actions (e.g. ACTION_REC_HOLD -> SNC_SAVE)
fixed: validation of mp3entry
Comment by x (vmh) - Saturday, 01 March 2008, 12:21 GMT
fixed: lrc conversion from utf16 to utf8 ('count' is not the size in bytes!)
Comment by x (vmh) - Wednesday, 26 March 2008, 21:20 GMT
fixed: adapting to new plugin api (do_menu())
changed: using function find_albumart() from api saves some code
Comment by x (vmh) - Thursday, 24 April 2008, 20:05 GMT
Keymapping has changed! Using the WPS key mapping context saves the work to define the keymapping for every player.
Unfortunately the simple push for Record button isn't used in the WPS context.
Comment by Jan (dumbo20) - Thursday, 01 May 2008, 18:06 GMT
So does the plugin work for all rockbox players now?
Comment by x (vmh) - Friday, 02 May 2008, 13:24 GMT
> So does the plugin work for all rockbox players now?
I would say yes even I didn't test it on other rockbox targets.

update:
added: set AB in AB-Menu to enable players with no AB buttons to set AB
added: functions goto_next_gap() and clean_blanks() in main menu

Comment by x (vmh) - Monday, 12 May 2008, 08:17 GMT
added: configurable (vertical) scrolling behaviour
Comment by x (vmh) - Monday, 12 May 2008, 17:39 GMT
removed: one of the scrolling options (unnecessary and confusing)
fixed: wrong scrolling for translations
Comment by Mike Miller (mikeage) - Wednesday, 14 May 2008, 07:30 GMT
Please note that as of r17492, the prototype for plugin_start needs to be:

enum plugin_status plugin_start(const struct plugin_api* api, const void* parameter)

Both parameters became const
Comment by x (vmh) - Sunday, 18 May 2008, 16:42 GMT
added: possibility to show Album Art below the lyrics for players with lcd height > width
changed: colors
Comment by Karl Anderson (D-Shadow) - Wednesday, 21 May 2008, 18:12 GMT
Great plugin, however I came across a few bugs.

The scroll-limit resets after every blank line in the lyrics. I'm attaching my lrc file and an animation of screen dumps for debugging purposes. Secondly, the line height of the highlighted line often increases unexpectedly.

I guess the bugs are purely cosmetic, but it would be great if you could fix them.
Comment by x (vmh) - Wednesday, 21 May 2008, 20:55 GMT
These two "bugs" are intended, thus I won't fix them. But you can easily fix them yourself.

>The scroll-limit resets after every blank line in the lyrics
My reasons for a reset:
1. A blank line as current line in the middle of the screen looks queer to me
2. A blank line usually represents an instrumental part and then a "new" beginning of the singing. The scrolling lyrics do the same.

if you don't want the reset, you can do as follows:
line 720:
if(IS_BLANK(id) ||
(id>0 && IS_BLANK(id-1)) ||
max_height < current_y0 + (snc->rows * fontheight) + tr_height)
current_y0 = scroll_y0;

remove the two IS_BLANK conditions:
if(max_height < current_y0 + (snc->rows * fontheight) + tr_height)
current_y0 = scroll_y0;

You also have to change the drawing line of the "countdown stars":
- turn the static variable current_y0 to a global variable
- use current_y0 instead of scroll_y0 in the drawing part of the "countdown stars"

> Secondly, the line height of the highlighted line often increases unexpectedly.
My reasons for the additonal space:
1. second highlighting effect (next to the color)
2. better distribution of the available space
(depending on the LCD height and the chosen font the empty space after the last lyrics line can be really large)
3. It has an animation effect if you place the scroll-limit at the bottom of the screen

If you don't want it just remove the following line in the code:
line 739: y+=((max_height-scroll_y0)%fontheight)>>1; /* additional space */
Comment by x (vmh) - Thursday, 22 May 2008, 18:15 GMT
fixed: wrong calculation of the maximum scrolling height (overlap with the "next song" line, see the picture from D-Shadow)
Comment by Karl Anderson (D-Shadow) - Friday, 23 May 2008, 08:20 GMT
That worked just fine... only had a problem with the position of countdown stars, but figured it out after a few tries.

Thanks a lot for your help.
Comment by x (vmh) - Sunday, 15 June 2008, 16:14 GMT
added: possibility to set a backdrop (./rockbox/rocks/apps/sncviewer_bd.bmp)
added: invert colors
changed: menu icons
Comment by x (vmh) - Sunday, 15 June 2008, 18:36 GMT
safer code ...
Comment by William (lee321987) - Tuesday, 17 June 2008, 01:48 GMT
I'm trying to compile this for my Sansa c200, but with no luck. I tried "make > output.txt", but none of the errors are not put into output.txt, so what I CAN say is that there are (at least) 294 errors that look like this:
sncviewer.c:6347: error: redefinition of 'store_line_'
sncviewer.c:1066: error: previous definition of 'store_line_' was here
sncviewer.c:6396: error: redefinition of 'store_line'
sncviewer.c:1119: error: previous definition of 'store_line' was here
They seem to all have the "previous definition" and "redefinition of" in them.
Then the last three lines are:
make[2]: *** [/home/Will/rockbox/build/apps/plugins/sncviewer.o] Error 1
make[1]: *** [rocks] Error 2
make: *** [build] Error 2
Any help would be much appreciated. Thank you.
Comment by x (vmh) - Tuesday, 17 June 2008, 15:55 GMT
It looks like you have applied the patch more than once. The file sncviewer.c doesn't have 6000 lines!
I think you can fix this manually:
- delete the file "sncviewer.c" in apps/plugins
- delete all the lines with "sncviewer" in the files apps/plugins/CATEGORIES and apps/plugins/SOURCES
- apply the patch

Always revert (patch -p0 -R < sncviewer_old.patch) the old patch before applying (patch -p0 < sncviewer_new.patch) the new one.
Comment by x (vmh) - Tuesday, 01 July 2008, 20:14 GMT
added: dictionary look up
Comment by x (vmh) - Sunday, 06 July 2008, 13:33 GMT
update:
- changed: clean blanks: the min blank length can be set; delete all uninitialized blanks
- fixed: reset scroll-limit if the font was changed
- changed: only reset scrolling if the length of the blank line is > 3s
Comment by x (vmh) - Sunday, 13 July 2008, 10:07 GMT
update:
changed: if the interval from 'auto cue' is set, then every song without lyrics will be auto cued
added: menu entry 'save'
Comment by x (vmh) - Sunday, 20 July 2008, 10:47 GMT
update:
changed: Album Art Icons
fixed: load translation messes up a-b position
changed: snc lyrics will be saved to lrc format
changed: same function for the 'next' button in both modes
and some other changes
Comment by x (vmh) - Sunday, 27 July 2008, 08:06 GMT
update:
changed: double click '|<<' button will change to the previous song in both modes
fixed: corrupted last lrc line
fixed: turn off 'auto cue' clears the loaded lyrics
Comment by Thomas Martitz (kugel.) - Sunday, 03 August 2008, 00:21 GMT
I'm getting a weird compiling error in line 169 ("expected identifier or '(' before string constant"). That's the file of the BOM const declaration. Commenting it out helps and seems not to be needed (there's a BOM define in the core: "#define BOM "\xef\xbb\xbf" " in misc.h).
Comment by Dominik Riebeling (bluebrother) - Sunday, 03 August 2008, 18:35 GMT
handling UTF-8 files should now be done using open_utf8() in misc.c ... this isn't exported to the plugin API yet.
(BTW, why add a trailing NULL byte to the BOM at all? You'd never check for that NULL anyway.)
Comment by Dominik Riebeling (bluebrother) - Thursday, 21 August 2008, 17:22 GMT
updated to follow changes in svn, and renamed the BOM variable. It still should use the core open_utf8() to handle the BOM issues though (but I haven't addressed this).

Besides, if someone is interested in getting this into svn, the file ignores the style guides quite a bit -- wrong indentation, constants written as macros etc.
Comment by Przemysław Hołubowski (p.h.) - Wednesday, 07 January 2009, 15:33 GMT
Please update the way rb->read_bmp_file() is called. It is out of sync with SVN and produces an error during making.
Comment by Teruaki Kawashima (teru) - Sunday, 05 April 2009, 12:40 GMT
I wrote a plugin to display .lrc file. sorry if it's no good to upload this here.

this plugin is what i think enough to display lyrics synchronized with song playing,
but simpler or less functional than sncviewer.
some code to handle audio is taken from apps/gwps* and apps/plugins/lib/playback_control.c
and some code to read .snc and read USLT/SYLT in id3 tag is taken from sncviewer. thanks.

although code to read snc and USLT/SYLT is included, I have no idea if they work correctry.

instruction (lrcplayer.txt) is also attached.
Comment by Ralph Soto (agalloch) - Wednesday, 27 May 2009, 00:13 GMT
snclrc (formerly sncviewer) is a Rockbox plugin for viewing synchronized lyrics in the lrc format
Found the source here http://yxz0123.yx.funpic.de/rockbox/index.html
All credit goes to the author of that site. (op?)
Comment by Ralph Soto (agalloch) - Thursday, 28 May 2009, 03:17 GMT
Fixed.
Comment by Teruaki Kawashima (teru) - Friday, 19 June 2009, 15:54 GMT
updated my patch.
Comment by Gman (Thecoolgman) - Thursday, 16 July 2009, 01:32 GMT
needs resync
Comment by Teruaki Kawashima (teru) - Wednesday, 22 July 2009, 13:14 GMT
synced.
Comment by anon ymous (sickasabat) - Thursday, 20 August 2009, 14:27 GMT
I'm not sure if this is the right place to be reporting problems with the lrcplayer.3.patch.
Would it be possible to update this patch to use the enhanced .lrc format ?
Can anyone give me any tips on how to go about helping with this patch?
While developing is there a quicker test cycle than recompiling into rockbox and testing on a device?
My apologies if this is posted in the wrong place.
Comment by Nick G (gersto) - Thursday, 27 August 2009, 04:37 GMT
just so those are aware an update was posted by Teruaki ina new flyspray: http://www.rockbox.org/tracker/task/10559?project=1&type=4&order=dateopened&sort=desc. Should we all assume that this one should be closed?
Comment by Teruaki Kawashima (teru) - Thursday, 27 August 2009, 07:09 GMT
I posted lrcplayer to new task because it seems confusing to have different two patches here.
i'd not close this task unless requested to close as snclrc seems to do more than what lrcplayer does and someone would want that feature.
Comment by Zach (enginerd) - Wednesday, 11 November 2009, 02:46 GMT
I am new to Rockbox and am trying to install either or both of these on my Toshiba Gigabeat S. I am confused on how to install these .patch files. If someone could help me out or direct me to a link that will walk me through it that would be great. Thanks
Comment by Shiloh Hawley (gree665) - Wednesday, 11 November 2009, 06:23 GMT
It involves compiling the source code:

http://www.rockbox.org/wiki/WorkingWithPatches
Comment by Bryan Childs (GodEater) - Saturday, 05 June 2010, 23:39 GMT
lrc viewer is now part of core rockbox - is this still required?
Comment by PurlingNayuki (yzflcyq) - Saturday, 17 July 2010, 13:50 GMT
Yes it is.
Well, you know, snclrc is a very, very nice thing and it's also very useful. In comparison with lrcplayer, it's more customizeful.
BTW, I ask 'he' to get the newer version of snclrc---nothing better than this!
   snclrc.c (128.8 KiB)

Loading...