Rockbox

  • Status Unconfirmed
  • Percent Complete
    0%
  • Task Type Bugs
  • Category User Interface → Themes
  • Assigned To No-one
  • Operating System Android
  • 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 sugerfunk - 2011-02-15

FS#11943 - EQ settings changed via Touchscreen setting_inc & setting_dec are not reflected in the sound output

EQ settings changed via Touchscreen WPS action setting_inc & setting_dec are not reflected in the sound output of the device even though they display having been changed in the menus. The changes are also not written to the main config file on disk so they do not "stick" after Rockbox has been rebooted.

example syntax to toggle EQ on/off (display DOES get toggled):
%T(148,520,54,46,setting_dec,eq enabled)
%T(148,520,54,46,&setting_inc,eq enabled)
%xl(E,eq.bmp,0,0)
%V(148,520,54,46,-)
%?St(eq enabled)<%xd(E)|>

I'm not sure if this problem is specific to Android or if the problem applies to all targets.
APK Build: Feb 4, 2011

The first part does not surprise me at all, by design settings_apply() is not called when the touchscreen button is pressed (if it did there would be lots of unnecessary disk access and possibly itwould stop/restart playback). The fix for this is to figure out the important settings here and get them to register a callback in settings_list.c so they can be applied without settings_apply() being called.

The change not getting written to config.cfg is worrying though.

It makes perfect sense that Rockbox wouldn't want to do a callback for most touchscreen presses.

But what about the possibility of adding one more additional (and optional) boolean parameter to the %T tag, exclusively for the setting_inc and setting_dec actions, which tells the WPS to do a settings_apply() callback only when true?
Perhaps this could make sense from an architectural perspective? It would allow EQ settings to be changed from the WPS, prevent any unnecessary callbacks by requiring an explicit request to do so, and retain backwards compatibility for all existing WPS screens.

One additional note: I just spent some time looking at settings_list.c & dsp.c and it appears that there is a function called dsp_set_eq() which is already defined and ready for callback (similar to the dsp_set_crossfeed callback in settings_list.c).

It is not defined in the eq_enabled declaration (and it appears that it might have been an unintentional omission):
OFFON_SETTING(F_EQSETTING, eq_enabled, LANG_EQUALIZER_ENABLED, false, "eq enabled", NULL)

Since the function is already defined in dsp.c, which is already imported, I think it would be straightforward to change it to:
OFFON_SETTING(F_EQSETTING, eq_enabled, LANG_EQUALIZER_ENABLED, false, "eq enabled", dsp_set_eq)

This looks exactly how "crossfeed" (also a boolean toggle) functions in the settings_list.c file:
OFFON_SETTING(F_SOUNDSETTING, crossfeed, LANG_CROSSFEED, false, "crossfeed", dsp_set_crossfeed)

I have yet to get a working build environment, and I haven't been able to test it, so please let me know if I'm missing something. :-)

that sounds about right

Added a patch to change settings_list.c

Any chance this patch can be applied to the main branch? It's only a simple change from NULL to a function callback, modelled after the other lines in the settings file, so I doubt it would break any functionality. I have been hoping for a while to toggle the graphic EQ on/off from the WPS screen. Thanks!

Any chance this patch can be applied to the main branch? It's only a simple change from NULL to a function callback, modelled after the other lines in the settings file, so I doubt it would break any functionality. I have been hoping for a while to toggle the graphic EQ on/off from the WPS screen. Thanks!

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing