Rockbox

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

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

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 .

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.

vmh commented on 2007-07-14 15:06

update:
- added: countdown marker
- some other small changes

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 ?

Add button mapping for the e200 target.

vmh commented on 2007-07-14 21:18

iRiver H10 button mapping added

Scroll wheel for volume fixed and documentation added for the e200 target.

Oops, something strange happened to attaching the documentation. Here it is again.

Oops, something strange happened to attaching the documentation. Here it is again.

vmh commented on 2007-07-15 06:35

I also got some warnings. I think the special characters (start and end tag) of a snc cause the problem.

vmh commented on 2007-07-20 14:30

update:
- added: cue sheet support (create (auto cue)/edit/save)

this update doesn't relate to lyrics, but it's useful for music podcast.

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?

vmh commented on 2007-07-21 16:33

"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.

vmh commented on 2007-07-22 15:48

fixed: displaying title and artist containing characters not covered by sysfont

TiMiD commented on 2007-07-23 01:49

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 !

vmh commented on 2007-07-23 05:16

- 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.

vmh commented on 2007-07-23 18:04

as suggested:
changed: use some predefined keymappings from pluginlib_actions.h
changed: use "struct screen"

vmh commented on 2007-08-06 18:36

update:
- adapting to the new rocks directory structure (rocks/apps)
- comments: // → /* */
- some optimisations

vmh commented on 2007-08-09 20:21

update:
- added: load translation file
- sncviewer.txt updated

vmh commented on 2007-08-16 19:29

fixed: some ab-repeat issues

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?

vmh commented on 2007-08-23 19:12

there is already a patch which reads USLT from an id3 tag
see [url]http://www.rockbox.org/tracker/2999[/url]

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!

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?

vmh commented on 2007-09-02 08:01

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?

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.

vmh commented on 2007-09-02 18:44

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?

You're right about the document; I didn't read carefully enough

http://mikeage.net/2007/09/03/batch-downloading-of-lyrics-for-mp3/

vmh commented on 2007-09-15 18:34

update:
fixed: ipod scrollbutton assignment
changed: save translation and peakmeter settings

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.

vmh commented on 2007-09-15 20:15

x5 doesn't have a button called BUTTON_MODE (#endif was at the wrong place)

vmh commented on 2007-09-16 16:31

update:
fixed: problems related to load translation file
fixed: replace /,\,?,: with _ on loading album art (path/<album>.bmp)

vmh commented on 2007-09-17 17:11

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

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?

vmh commented on 2007-09-17 17:31

Do you mean the sncviewer.txt? It's only a documentation. Only the sncviewer.patch is needed for building.

vmh commented on 2007-09-18 16:44

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)

vmh commented on 2007-09-22 12:02

fixed: don't add time offset to not initialized time tags
fixed: unify behaviour for leaving plugin (quit plugin, audio stopped, usb connected)

vmh commented on 2007-10-01 11:38

some fixes …

vmh commented on 2007-10-12 16:41

changed:
- save temporary tagged txt files in lrc-format
- reorder some functions

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!

vmh commented on 2007-10-16 18:18

no problem, it's not a big thing to add it.

update:
added: delete lyrics entry in the menu

nice, thanks!

i'll try it out on my commute tomorrow (don't worry, I don't drive myself to work <g>)

vmh commented on 2007-10-26 17:24

update:
fixed: doesn't notice track change in the recent build
changed: translation icon

vmh commented on 2007-11-06 20:10

added: USLT support
changed: capitalize enums, rename some variables, …

vmh commented on 2007-11-17 09:25

changed: track change handling, some other internal functions

vmh commented on 2008-01-23 09:31

fixed: backlight
changed: simplify track change handling

vmh commented on 2008-01-24 13:16

fix a bug in yesterday's version: adjusting the volume will lead to a crash.

vmh commented on 2008-02-11 13:13

updated: sncviewer.txt
changed: scrolling in editor
fixed: some bugs

vmh commented on 2008-02-13 11:06

changed: rename all actions (e.g. ACTION_REC_HOLD → SNC_SAVE)
fixed: validation of mp3entry

vmh commented on 2008-03-01 12:21

fixed: lrc conversion from utf16 to utf8 ('count' is not the size in bytes!)

vmh commented on 2008-03-26 21:20

fixed: adapting to new plugin api (do_menu())
changed: using function find_albumart() from api saves some code

vmh commented on 2008-04-24 20:05

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.

So does the plugin work for all rockbox players now?

vmh commented on 2008-05-02 13:24
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

vmh commented on 2008-05-12 08:17

added: configurable (vertical) scrolling behaviour

vmh commented on 2008-05-12 17:39

removed: one of the scrolling options (unnecessary and confusing)
fixed: wrong scrolling for translations

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

vmh commented on 2008-05-18 16:42

added: possibility to show Album Art below the lyrics for players with lcd height > width
changed: colors

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.

vmh commented on 2008-05-21 20:55

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 */

vmh commented on 2008-05-22 18:15

fixed: wrong calculation of the maximum scrolling height (overlap with the "next song" line, see the picture from D-Shadow)

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.

vmh commented on 2008-06-15 16:14

added: possibility to set a backdrop (./rockbox/rocks/apps/sncviewer_bd.bmp)
added: invert colors
changed: menu icons

vmh commented on 2008-06-15 18:36

safer code …

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.
vmh commented on 2008-06-17 15:55

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.

vmh commented on 2008-07-01 20:14

added: dictionary look up

vmh commented on 2008-07-06 13:33

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

vmh commented on 2008-07-13 10:07

update:
changed: if the interval from 'auto cue' is set, then every song without lyrics will be auto cued
added: menu entry 'save'

vmh commented on 2008-07-20 10:47

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

vmh commented on 2008-07-27 08:06

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

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).

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.)

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.

p.h. commented on 2009-01-07 15:33

Please update the way rb→read_bmp_file() is called. It is out of sync with SVN and produces an error during making.

teru commented on 2009-04-05 12:40

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.

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?)

Fixed.

teru commented on 2009-06-19 15:54

updated my patch.

needs resync

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.

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?

teru commented on 2009-08-27 07:09

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.

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

It involves compiling the source code:

http://www.rockbox.org/wiki/WorkingWithPatches

lrc viewer is now part of core rockbox - is this still required?

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...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing