- Status New
- Percent Complete
- 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
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 .
Loading...
Available keyboard shortcuts
- Alt + ⇧ Shift + l Login Dialog / Logout
- Alt + ⇧ Shift + a Add new task
- Alt + ⇧ Shift + m My searches
- Alt + ⇧ Shift + t focus taskid search
Tasklist
- o open selected task
- j move cursor down
- k move cursor up
Task Details
- n Next task
- p Previous task
- Alt + ⇧ Shift + e ↵ Enter Edit this task
- Alt + ⇧ Shift + w watch task
- Alt + ⇧ Shift + y Close Task
Task Editing
- Alt + ⇧ Shift + s save task
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.
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.
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.
I also got some warnings. I think the special characters (start and end tag) of a snc cause the problem.
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?
"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.
fixed: displaying title and artist containing characters not covered by sysfont
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 !
- 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.
as suggested:
changed: use some predefined keymappings from pluginlib_actions.h
changed: use "struct screen"
update:
- adapting to the new rocks directory structure (rocks/apps)
- comments: // → /* */
- some optimisations
update:
- added: load translation file
- sncviewer.txt updated
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?
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?
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.
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/
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.
x5 doesn't have a button called BUTTON_MODE (#endif was at the wrong place)
update:
fixed: problems related to load translation file
fixed: replace /,\,?,: with _ on loading album art (path/<album>.bmp)
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?
Do you mean the sncviewer.txt? It's only a documentation. Only the sncviewer.patch is needed for building.
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)
fixed: don't add time offset to not initialized time tags
fixed: unify behaviour for leaving plugin (quit plugin, audio stopped, usb connected)
some fixes …
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!
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>)
update:
fixed: doesn't notice track change in the recent build
changed: translation icon
added: USLT support
changed: capitalize enums, rename some variables, …
changed: track change handling, some other internal functions
fixed: backlight
changed: simplify track change handling
fix a bug in yesterday's version: adjusting the volume will lead to a crash.
updated: sncviewer.txt
changed: scrolling in editor
fixed: some bugs
changed: rename all actions (e.g. ACTION_REC_HOLD → SNC_SAVE)
fixed: validation of mp3entry
fixed: lrc conversion from utf16 to utf8 ('count' is not the size in bytes!)
fixed: adapting to new plugin api (do_menu())
changed: using function find_albumart() from api saves some code
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?
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
added: configurable (vertical) scrolling behaviour
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
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.
These two "bugs" are intended, thus I won't fix them. But you can easily fix them yourself.
if you don't want the reset, you can do as follows:
line 720:
remove the two IS_BLANK conditions:
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"
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 */
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.
added: possibility to set a backdrop (./rockbox/rocks/apps/sncviewer_bd.bmp)
added: invert colors
changed: menu icons
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
make[2]: * [/home/Will/rockbox/build/apps/plugins/sncviewer.o] Error 1
make[1]: * [rocks] Error 2
make: *** [build] Error 2
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.
added: dictionary look up
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
update:
changed: if the interval from 'auto cue' is set, then every song without lyrics will be auto cued
added: menu entry 'save'
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
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.
Please update the way rb→read_bmp_file() is called. It is out of sync with SVN and produces an error during making.
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.
updated my patch.
needs resync
synced.
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?
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!