This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#6331 - Voice and features for FM radio
Attached to Project:
Rockbox
Opened by Stephane Doyon (sdoyon) - Wednesday, 15 November 2006, 04:00 GMT+2
Last edited by Stephane Doyon (sdoyon) - Wednesday, 07 November 2007, 03:15 GMT+2
Opened by Stephane Doyon (sdoyon) - Wednesday, 15 November 2006, 04:00 GMT+2
Last edited by Stephane Doyon (sdoyon) - Wednesday, 07 November 2007, 03:15 GMT+2
|
DetailsThis patch adds voice to the FM radio, re-does some menus and adds a few
new functions. First of all, someone made it possible for the voice to talk while the radio is playing. I'm pretty sure that wasn't working two months ago. Big thanks to whoever did that. Requires P#6159 and P#6236. First I had all the splashes and dialogs talk (just use ID2P, see P#6159). Just that helps a lot. First thing that happens when going into the radio screen for the first time is the auto-scan dialog, which was silent. I have it say the tune mode (scan vs preset) when it changes. If the voice file preference is to say numbers, I have it say the index of the preset, otherwise if the setting is for spelling and the preset name is something else than the default auto-scan name, I have it spell that. Obviously it's convenient only for very short names or call letters. If the preference is for spelling but there's no name, or if we're in scan mode, then I have it also say the current frequency. I'm assuming it's OK to speak only one decimal place instead of the two that are displayed. But then I came to the radio_menu and the presets menu. The radio_menu has three unusual elements: the menu item contains both a field name and a value, and clicking it changes the value in place. They are the entries for force mono mode, region, and scan mode. I added ordinary full-blown submenus for those options. Until I can get some feedback, I have it use the submenus if the talk_menu setting is enabled, and the original method otherwise. At least for force mono and region, I think I would prefer having a submenu, for uniformity and the ability to cancel a change. And those two are not changed often. I'm not sure what to make of the button preprocessor function, and it does say #if 0 /* this screen needs fixing! */ The presets menu is even more of a problem: menu entries are preset names. I need to have those spelled. The menu structure seems a bit convoluted to me. There's this preset_menu_items array that needs to be constantly rebuilt with copies of the preset names from presets]. There's some elaborate manipulations to keep both lists in sync. Oh and there's that button preprocessor function, the purpose of which I don't really get. I chose to reimplement the presets menu using a gui_synclist. An alternative might have been to come up with some menuing infrastructure to allow arbitrary speech for each menu entry, perhaps through some sort of voice callback. The gui_synclist made it easier to add a few new functions however. And the callback avoids the duplicated preset lists. cvs log tells me this menuing code stuff isn't old, so I may be taking apart fairly recent code here. Comments appreciated, or if anyone wants to educate me wrt RB menus, or has a good idea how to handle complex voice output for menu entries... Some other menu entries in RB that can't be spoken are: recording directory, simple equalizer. I added two new functions to the preset context menu: -Ability to move a preset, -Ability to insert a new preset at a given spot in the list. I was certainly influenced by the playlist_viewer which I just worked with. I even put in the icons to indicate the currently playing preset and the preset being moved. Oh and I indicate both the preset name and frequency. If a sighted person could tell me whether this all works right, it would be appreciated. And in the radio_menu, I added a new function to sort the presets by frequency. Oh and I fixed a little glitch when deleting the last preset: access beyond array bounds, uses uninitialized data, tunes to frequency to 0.0 which seems to confuse my hardware... |
This task depends upon
Closed by Stephane Doyon (sdoyon)
Wednesday, 07 November 2007, 03:15 GMT+2
Reason for closing: Accepted
Wednesday, 07 November 2007, 03:15 GMT+2
Reason for closing: Accepted
* It only adds speech and doesn't include the additional features in the original; imo, these should probably be implemented separately now that the code doesn't have to be modified so much to add speech.
* It does not make an exception to spelling the preset name if it is the default name assigned during autoscan, as I feel that there should not be an exception here. I understand the reason behind it, but it is then somewhat different to what is reflected by the rest of the UI. Perhaps autoscan should just assign blank preset names and only show the frequency for blank preset names? Hmm, I don't quite like that idea, but I think the UI as a whole needs to handle this differently, not just the voice. (I know the original patch did change the UI a bit in this regard.)
* Currently, only the preset name is spoken in the presets menu and not the frequency. The original patch modified both the UI and voice to include the frequency. This is easy enough to do, but I'm trying to leave the UI out of this patch for now.
Thanks very much for the original idea and patch, Stephane. I've been wanting to do this for a long time but hadn't gotten around to it. The new radio code is certainly much easier to work with! :)
Btw, I don't entirely disagree with the ideas you have removed. I'm just trying to separate this out a bit for now so we can get it included and then work on some of the functional changes.
anyone would ever look at this.
Yes I do agree that I threw in too many things together in that original
patch, and it was pretty intrusive too. Sorry. Most of my other patches
had attracted little or no interest, so at that point I was really doing
it for myself, so I did not bother to split things neatly.
So thanks for looking at this! I'll try to find some time to update my
stuff...
The French translation for example would probably have the words the other way round ("mode preset", and "mode scan").
1) No need for talk_preset() to return a boolean, AFAICT, so we can simplify
the logic slightly.
2) Also you have bool talk = false; on entering the radio_screen(), whereas
I would generally have the current setting spoken right away, so I'd
initialize it to true. Also that gives a nice feedback on completion
of the first autoscan (otherwise the user is left kind of dangling).
I understand your point about not changing the UI, but I must say
that spelling out the frequencies in the preset names and the MHz letters
really doesn't sound too good, and it's very verbose, it becomes
somewhat annoying.
Perhaps I should write up the small required UI change in a separate patch.
Or perhaps the radio screen should only speak in scan mode, which is
presumably when you're looking to match a frequency. You often don't
need to be told the preset name, and I find it can get annoying when
you know perfectly well what it is and yet the voice drowns out the
station for a couple of seconds while it tells you again. Or you don't
care what it is because you're just looking for some good music on any
station. I'm normally one for total information access, but the trick
is in this case, one can just enter the preset menu to find out what
the current preset is. So it would take an extra step, but the info
would still be there. Thoughts?
"nine seven dot seven zero ehm aytch zee".
radio-preset-default-empty.diff changes the default preset name to the
empty string, and modifies the preset menu list's get_item callback to
substitute LANG_FM_DEFAULT_PRESET_NAME when the empty string is
encountered. So nothing changes in the on-screen UI, except when you go
to edit a preset name: if it was the default, you start with an empty
string, which is good because you don't have to erase it.
Here's a new radio_voice.patch. It recognizes when a preset name string
is empty, and in that case it speaks the frequency instead, as a number
(rather than spelling it).
This version of radio_voice.patch was also made to use my voice callback
for synclists: P7774. So it now depends on that.
I think it is from the function:
void talk_preset(int preset, bool fallback, bool enqueue)
Just my opinion...
I would do it. But you don't want me to update your patches because my formatting isn't great yet. And second I am using r15239 and don't want to resync everything. I am going to learn linux and stuff so I probably won't update my rockbox for a while. Or even write patches...
O yeah re the talking presets you would have to remove the call to that function as well...
Just removed a call to the talking preset function.
Commented it out actually. If I was going to upload it I would need to do it properly. And remove the actuall function and remove the calls to it. Not comment stuff out...
I have still got the function here just commented out the calls.
I will see how it goes and if I like it might upload a new version.
every station change is annoying:
-because when you've identified one station, you often know what the
neighboring stations are,
-because often you're just looking for good music on any station.
OTOH on some occasions it's really nice to be able to identify what
station you are listening to or to be able to find a particular station.
So an idea: speak the frequency and/or preset name when switching stations
but only when the radio is paused.
Patch attached.