Rockbox

Tasklist

FS#2924 - Recording screen Gain adjustments (Upgrade of patch1403437)

Attached to Project: Rockbox
Opened by Martin Scarratt (mmmm-) - Tuesday, 17 January 2006, 18:37 GMT
Last edited by Martin Scarratt (mmmm-) - Thursday, 09 February 2006, 14:24 GMT
Task Type Patches
Category
Status Closed
Assigned To No-one
Operating System
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

This patch (which is a heavily modified version of
Petur's original) removes the option of separate gain
controls and makes all gain changes appear to be the
same (whereas actually it's all very complicated!) so
for the end-user it is obvious what to do.

No matter what mode you are in the gain will change in
steps of 0.5db when increasing and 0.5db when decreasing.

The analogue gain is always at it's highest possible
for the setting so that maximum quality is preserved.

I've included a Developer version so that you can see
what is going on "behing the scenes" if you like.

enjoy

Martin Scarratt
This task depends upon

Closed by  Martin Scarratt (mmmm-)
Thursday, 09 February 2006, 14:24 GMT
Reason for closing:  
Comment by Martin Scarratt (mmmm-) - Tuesday, 17 January 2006, 22:16 GMT

Bugfix!
Comment by Martin Scarratt (mmmm-) - Tuesday, 17 January 2006, 22:31 GMT

Last bugfix of the day! :)
Comment by Martin Scarratt (mmmm-) - Wednesday, 18 January 2006, 07:36 GMT

First bugfix of the next day!
this should be it now! :D
Comment by Peter D'Hoye (petur) - Wednesday, 18 January 2006, 23:42 GMT

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...

Comment by Martin Scarratt (mmmm-) - Thursday, 19 January 2006, 16:16 GMT

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).
Comment by Peter D'Hoye (petur) - Friday, 20 January 2006, 21:49 GMT

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...
Comment by Peter D'Hoye (petur) - Saturday, 21 January 2006, 01:04 GMT

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.

Comment by Martin Scarratt (mmmm-) - Sunday, 22 January 2006, 11:23 GMT

Bye bye literals...
and some general housework! :-#)
Comment by Martin Scarratt (mmmm-) - Sunday, 22 January 2006, 12:14 GMT

Update for latest CVS
Comment by Martin Scarratt (mmmm-) - Monday, 23 January 2006, 16:58 GMT

Mono change for Mic in (makes it MUCH simpler!)
Tidy up...
Comment by Peter D'Hoye (petur) - Monday, 23 January 2006, 19:40 GMT

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 :)
Comment by Peter D'Hoye (petur) - Monday, 23 January 2006, 22:38 GMT

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]
Comment by Martin Scarratt (mmmm-) - Tuesday, 24 January 2006, 14:09 GMT

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! :)
Comment by James Teh (jteh) - Wednesday, 01 February 2006, 02:30 GMT

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. :)
Comment by Peter D'Hoye (petur) - Thursday, 02 February 2006, 18:03 GMT

I'm going to separate the digital gain for the two input
sources so that each uses its own gain
Comment by Peter D'Hoye (petur) - Friday, 03 February 2006, 11:59 GMT

new patch attached to my tracker entry

Loading...