Rockbox

The last version of the original patch no longer applies at all, as there have been some major changes to the radio code, one of which is that the presets menu does now use a synclist. I chose to reimplement speech for the radio screen. It is based on the ideas of the original patch but does things a little differently. Differences in functionality include:
* 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.

   radio_voice.patch (8.5 KiB)

Err, I meant “don’t entirely disagree with the ideas *I* have removed”. Sorry.

I’m VERY glad to see some interest for this. I had quite lost hope that
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…

Synced with latest svn (r14229). One difference in this version of the patch is that “Scan mode” and “Preset mode” are now spoken as two strings: “Scan” or “Preset” and then “Mode”. This does introduce a pause and some strange inflection, but I don’t really see the point of adding two new strings for this when there is a suitable alternative. (LANG_PRESET is now used in a more unified manner, which is the reason that this has changed.)

   radio_voice.patch (5 KiB)

One of the problems with splitting sentences into composite parts and joining them together is that different languages order words differently to say the same thing compared to english.

The French translation for example would probably have the words the other way round (”mode preset”, and “mode scan”).

This did occur to me. There are two solutions: just speak “Scan” and “Preset” and don’t bother to speak “Mode” or implement two new lang strings for the two modes. I am happy to do either. The first is arguably not as intuitive for newer users, yet less verbose for those of us who are familiar with it.

Reinstated the “Scan mode” and “Preset mode” voice strings.

   radio_voice.patch (5.7 KiB)

Synced to r14260.

   radio_voice.patch (5.7 KiB)

Synced to r14391.

   radio_voice.patch (5.6 KiB)

Here’s a slightly modified version:

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?

   radio_voice.patch (5.7 KiB)

The spelling of the default preset name is really too annoying:
“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.

   radio-preset-default-empty.di... (1.2 KiB)
   radio_voice.patch (7.2 KiB)

It is a bit annoying when you have added a preset and you scrol over it. The voice tells you and you already know what it is… I am not complaining but do you think we could remove the preset talking code?
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…

Ok modified a big patch sdoyon sent me so can’t upload my changes. Might later if I think they’re useful.
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.

OK so voicing the frequency and/or preset name in the radio screen on
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.

   radio_voice.patch (5.7 KiB)