This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#7432 - sncviewer - a plugin for viewing synchronised lyrics on player screen
Attached to Project:
Rockbox
Opened by Eddy (bascule) - Saturday, 14 July 2007, 10:59 GMT+2
Last edited by Eddy (bascule) - Saturday, 14 July 2007, 11:09 GMT+2
Opened by Eddy (bascule) - Saturday, 14 July 2007, 10:59 GMT+2
Last edited by Eddy (bascule) - Saturday, 14 July 2007, 11:09 GMT+2
|
DetailsThis 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 . |
sncviewer.txt
This task depends upon
Operating instuctions are fine.
- added: countdown marker
- some other small changes
- added: cue sheet support (create (auto cue)/edit/save)
this update doesn't relate to lyrics, but it's useful for music podcast.
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?
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.
- 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 !
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.
changed: use some predefined keymappings from pluginlib_actions.h
changed: use "struct screen"
- adapting to the new rocks directory structure (rocks/apps)
- comments: // -> /* */
- some optimisations
- added: load translation file
- sncviewer.txt updated
see [url]http://www.rockbox.org/tracker/2999[/url]
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!
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?
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?
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.
Your script sounds interesting. Where can I download it?
http://mikeage.net/2007/09/03/batch-downloading-of-lyrics-for-mp3/
fixed: ipod scrollbutton assignment
changed: save translation and peakmeter settings
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.
fixed: problems related to load translation file
fixed: replace /,\,?,: with _ on loading album art (path/<album>.bmp)
- 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
fixed: unify behaviour for leaving plugin (quit plugin, audio stopped, usb connected)
- save temporary tagged txt files in lrc-format
- reorder some functions
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!
update:
added: delete lyrics entry in the menu
i'll try it out on my commute tomorrow (don't worry, I don't drive myself to work <g>)
fixed: doesn't notice track change in the recent build
changed: translation icon
changed: capitalize enums, rename some variables, ...
changed: simplify track change handling
changed: scrolling in editor
fixed: some bugs
fixed: validation of mp3entry
changed: using function find_albumart() from api saves some code
Unfortunately the simple push for Record button isn't used in the WPS context.
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
fixed: wrong scrolling for translations
enum plugin_status plugin_start(const struct plugin_api* api, const void* parameter)
Both parameters became const
changed: colors
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.
>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 */
Thanks a lot for your help.
added: invert colors
changed: menu icons
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.
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.
- 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
changed: if the interval from 'auto cue' is set, then every song without lyrics will be auto cued
added: menu entry 'save'
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
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
(BTW, why add a trailing NULL byte to the BOM at all? You'd never check for that NULL anyway.)
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.