Rockbox

Tasklist

FS#10680 - Allow %?Sp<...> to be used as a conditional

Attached to Project: Rockbox
Opened by Junio C Hamano (gitster) - Friday, 16 October 2009, 19:45 GMT
Last edited by Frank Gevaerts (fg) - Monday, 09 November 2009, 17:44 GMT
Task Type Patches
Category User Interface
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

When there are two choices (i.e. boolean), choose the first one
if the pitch is different from the normal value, and choose the
second one if the pitch is the same as the normal value.

When there are more than two choices (i.e. enum), the left half
of the choices are used to show 0..normal range, and the right
half of the choices are used to show values over that range.

2 items: %?Sp<0..99 or 101..infinity|100>
3 items: %?Sp<0..99|100|101..infinity>
4 items: %?Sp<0..49|50..99|100|101..infinity>
5 items: %?Sp<0..49|50..99|100|101..149|150..infinity>
6 items: %?Sp<0..33|34..66|67..99|100|101..133|134..infinity>
7 items: %?Sp<0..33|34..66|67..99|100|101..133|134..167|167..infinity>

This will allow two expected use cases:

(1) %?Sp<%Sp> to show the pitch only when playing at a modified pitch.

(2) %?Sp<%xdAa||%xdAb> to show an icon of Darth Vader (subpicture 1
of image A) when playing at a lower pitch than the original, and to
show an icon of a chipmunk (subpicture 2 of image A) when playing at
a higher pitch than the original. When playing at a normal pitch,
nothing is shown.
This task depends upon

Closed by  Frank Gevaerts (fg)
Monday, 09 November 2009, 17:44 GMT
Reason for closing:  Accepted
Additional comments about closing:  committed as r23589
Comment by Junio C Hamano (gitster) - Saturday, 17 October 2009, 06:21 GMT
When there are two choices (i.e. boolean), choose the first one if the
pitch is different from the normal value, and choose the second one if the
pitch is the same as the normal value.

When there are more than two choices (i.e. enum), the left half of the
choices are used to show 0..normal range, and the right half of the
choices are used to show values over that range. The last entry is used
when it is set to the normal setting, following the rockbox convention to
use the last entry for special values.

2 items: %?Sp<0..99 or 101..infinity|100>
3 items: %?Sp<0..99|101..infinity|100>
4 items: %?Sp<0..49|50..99|101..infinity|100>
5 items: %?Sp<0..49|50..99|101..149|150..infinity|100>
6 items: %?Sp<0..33|34..66|67..99|101..133|134..infinity|100>
7 items: %?Sp<0..33|34..66|67..99|101..133|134..167|167..infinity|100>

This will allow two expected use cases:

(1) %?Sp<%Sp> to show the pitch only when playing at a modified pitch.

(2) %?Sp<%xdAa|%xdAb|> to show an icon of Darth Vader (subpicture 1
of image A) when playing at a lower pitch than the original, and to
show an icon of a chipmunk (subpicture 2 of image A) when playing at
a higher pitch than the original. When playing at a normal pitch,
nothing is shown.

---
As suggested by kugel, the v2 patch makes the entry for 100% the last in the list.
Comment by Marianne Arnold (pixelma) - Thursday, 22 October 2009, 05:28 GMT
I'm a bit confused by the title because it already is possible to use %Sp conditionally... ah you mean the conditional to be used conditionally.

How does this interact with the current functionality of %?Sp? I mean if someone already uses it, would it break his WPS somewhat (especially if the "normal" value is put at the end?

There is something else I wonder about and where I would like the functionality and where the current %?Sp also doesn't cope - which is negative values in the settings. A real world example might be better: I wanted to make a WPS for "line out use" which should indicate if the sound settings are not "flat" - I couldn't make it work for bass and treble. After a quick look into the code I found a wrong assumption ("values start at 0") but for bass and treble they can be negative. How does this patch deal with that, or is there something that should be fixed in trunk?
Comment by Junio C Hamano (gitster) - Friday, 23 October 2009, 19:31 GMT
Well, I didn't know %?Sp was already supported and did something different. Given that %Sp were showing "1000" not even "100" for normal pitch before I patched it recently, I highly suspect that anybody was really using it.
Comment by Junio C Hamano (gitster) - Sunday, 25 October 2009, 07:56 GMT
I have been _very_ confused by Marianne's comment, especially about "treble" or "bass". Is it possible that Marianne misunderstood that I was talking about %?St (notice 't')? This patch and the discussion is about %?Sp (p stands for "pitch").

Besides not having an documented use in the manual (B.8 only lists %Sp not conditional %?Sp),
as far as I can tell, without the patch in this FS entry, the code does not do anything for %?Sp.

%?St does have a documented use (B.14 in the manual) and if Marianne's above comment was due to misunderstanding that I was talking about it, perhaps we can disregard that objection and apply this, together with #10681?


Comment by Junio C Hamano (gitster) - Wednesday, 04 November 2009, 18:41 GMT
As discussed on IRC (http://www.rockbox.org/irc/log-20091028#21:09:22), Marianne's comment above turns out to be unrelated to what this patch does. Can we close this (and its companion  FS#10681 ) by applying them?

Loading...