Rockbox

Bugfix!

Last bugfix of the day! :)

First bugfix of the next day!
this should be it now! :D

my comments so far (haven't actually tested it):
- it depends on knowledge how many dBs steps for each gain
type… - watch out for the code police (tabs, c++ style comments,
80+ chars on a line, …)
- I don't know if I really want analog and digital gain
active at the same time to give that fine resolution. I was
happy with the fact that while inside the analog range,
digital gain was always zero…

Fixed some strange behaviour, tidied code and comments and
better handling of max/min boundaries with mic in.

Ta for the feedback Petur. You really have to try it to see
what's going on…!

(re: comment 1) Honestly… you don't need any prior
knowledge, it's all done behind the scenes for you.

(re: comment 2)tidied it a bit.

(re: comment 3) I think the fine resolution is handy and
also present in the original. Therefore there is no loss of
functionality. Also the change in digital gain within the
analogue range is minimal (3dbs max in stereo 2dbs in mono).

re: comment1: I mean the code assumes a lot and uses
literals (like assuming 6 digital gain units are 1 analog
gain unit), so if somebody wants to use it on another
platform, they'll have a hard time….

I think the way to go is the other way around:
1. Set out the dB range and minimum step.
2. Let user change dB
3. convert this dB value to the best combination of analog
and digital gain and send that to the hardware

The only problem is that the dB steps are < 1. So we might
need to work at a scale * 10

The end result will be much simpler code that is very
flexible.

I'll have a go at it…

I wanted to attach my alternative code but

- I'm not so convinced anymore that it is better: it also
depends too much on literals and the code isn't more compact

- I failed to get it debugged before EOD (end of day), will
try again later just to test the principle.

Bye bye literals…
and some general housework! :-#)

Update for latest CVS

Mono change for Mic in (makes it MUCH simpler!)
Tidy up…

Nice.

The code needs to be a little bit more robust. After
upgrading to this version, analog and digital gain can be
anything, and things like

global_settings.rec_decimator_left_gain == ana_line_size

won't quite give the desired effects ;)

Also don't understand your definition of 'tidy' :o)
your patch also fails against the latest CVS version of
english.lang

And have a look at the code at line 222 and following in
recording.c: UDA1380 check is no longer needed. I would
also implement the combined L+R gain setting for the other
targets.

Almost there :)

patch updated… developer output is still i the code in
comment…

changed:
- always maximum analog gain (no more negative digital gain
when analog is positive)
- combined L/R gain now also for the other targets
- minor fixes
- tidied the code more
- made compatible with latest CVS version

[attached to my patch in the tracker]

Good one Petur… I'll upload it here too…

Peturs modification of my modification of Petur's patch! :D

I think we are finally there! :)

Reading through the patch (I haven't actually tested it
yet, though), it appears that decimator gain, mic analog
gain and line analog gain are still all saved as separate
settings. This means that,for example, changing the line
gain will have some effect on the saved setting for mic
gain because the decimator setting changes for both. Given
the combined gain provided by this patch, it would seem
more logical to save the combined gain for mic and the
combined gain for line separately so that one is not
affected by the other. This would probably involve making
the decimator and analog gains transparent to the actual
recording code, though; i.e. implement a function which
takes a dB value (possibly * 10) and sets the analog and
decimator gains accordingly. I think this is what Petur was
proposing in this comment:
"1. Set out the dB range and minimum step.
2. Let user change dB
3. convert this dB value to the best combination of analog
and digital gain and send that to the hardware"
Petur later noted that:
"I'm not so convinced anymore that it is better: it also
depends too much on literals and the code isn't more
compact"
I may take a crack at this later, though I don't have a
cross compiler set up to test it yet and my understanding
of the code is still minimal at best. :)

I'm going to separate the digital gain for the two input
sources so that each uses its own gain

new patch attached to my tracker entry