Audio gain not working

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Adjust Settings >> Advanced Options >> Earpiece Gain
2. Observe audio volume does not change no matter what the setting

What is the expected output? What do you see instead?
Expect audio volume to change as per setting.

Please use labels and text to provide additional information.
I have prepared a patch, but see additional notes below.

Original issue reported on code.google.com by jropal...@gmail.com on 10 Feb 2010 at 6:47

[GoogleCodeExporter]

In looking into this, I found it very hard to follow the code in
RtpStreamReceiver.java and RtpStreamSender,java.

As I worked to find the problem, I made a series of changes to the code to tidy 
it
up, and to make it clearer what is going on.  In doing so, I discovered a 
number of
other problems with this code.

The patch attached therefore fixes a number of problems:
        - cosmetic code cleanups:
                - fix indentation
                - remove "TAB SPACE" sequences
                - remove "SPACE EOL" sequences
                - add some line breaks between code sections

        - commentary
                - added comments to explain what each code section does

        - code simplifications that don't change app behavior:
                - changed "ring", the buffer slot pointer, to be a simple index,
                  rather than a byte offset
                - G711.[au]law2linear methods now take buffer offset
                - calc() split into separate methods: silence() and audgain()
                - calc1(), calc5(), calc10() removed; subsumed into audgain()

        - code changes that do change app behavior:
                - do comfort noise addition, even for speakerphone
                - a/v sync delay no longer uses incorrect buffer size (in
ui/VideoCamera.java)
                - alert tone placed in current buffer slot, not slot 0
                - audio gain control function added for received audio

Since the patch is a little long, I am submitting it here for review prior to
committing it.

Original comment by jropal...@gmail.com on 10 Feb 2010 at 6:52

Attachments:

• sipdroid-bigFix-II-20100210.diff

[GoogleCodeExporter]

Original comment by jropal...@gmail.com on 10 Feb 2010 at 7:47

Attachments:

• sipdroid-i314_2.diff

[GoogleCodeExporter]

Thanks for discussing this first.

Your code simplifications actually change app behavior. Code was splitted for 
optimization purposes. The modified clipping checks won't work.

I propose that you undo them to focus on the intended code changes thus 
avoiding to 
introduce any bugs to a well tested code. Also please post a diff file ignoring 
any 
white space changes to make it easier to read. I would even leave the white 
spaces 
completely untouched to make the version control as transparent as possible. 
This is  
more important than the white spaces itself.

Back to the headline of this bug report: earpiece gain should be set as volume 
in 
restoreVolume(). This works with my phone.

Original comment by pmerl...@googlemail.com on 10 Feb 2010 at 9:32

[GoogleCodeExporter]

Nothing is committed yet; I have the changes in a separate copy here and won't 
commit
until things are agreed.

Interesting that the audio gain is working on your phone.  Not working on mine. 
Nexus One here.

I looked at restoreVolume() and saw that it sets the gains of STREAM_RING and
STREAM_MUSIC but not the gain of STREAM_VOICE_CALL, so figured that code was for
something else.  Perhaps STREAM_VOICE_CALL is also needed here?  But I then 
looked at
how the micgain was handled in RtpStreamSender and decided to duplicate that 
solution
for the earpiece gain in RtpStreamReceiver.  On my N1, the audio gain does now 
work.
 But if you think that adding STREAM_VOICE_CALL in restoreVolume might be a simpler
solution, I can try that.  Or replacing STREAM_MUSIC with STREAM_VOICE_CALL?

As for calc(), calc1(), calc5() and calc10(), on examining them they all do the 
same
thing using (almost) the same clipping value:
    calc:    6550*5    32750
    calc1:   no clipping
    calc5:   16350<<1  32700
    calc10:  8150<<2   32600
I could not see why these different values were being used, and it did not make
sense: gain*5 clip at one value, gain*2 clip at a lower value, gain*4 clip at 
an even
lower value.  Why?  So, in combining the functions, I made them all 32767.  On 
my
phone, this simpler code appears to work fine.

The other bugs fixed here also need review:
    - outgoing comfort noise is only added when NOT in call
      (speakermode == MODE_NORMAL) but not when actually in a call
    - a/v sync uses fixed buffer multiple of 1024, even when frame_size is 160
    - alert tone always placed in buffer slot 0 rather than current buffer
      slot (works but this is bad because recorded audio might be xmitted later)

As for the whitespace fixing, I can do that later, no problem.  I will do that 
as a
separate commit and also add the comments as a separate commit too.  No point in
losing that work.

I have no problem taking this large patch and splitting it into separate diffs, 
each
ultimately done as a separate commit.  They'd be:
    - whitespaces and indentation (no change to code function)
    - add more comments (no change to code function)
    - fix comfort noise generation
    - simplify/merge the four audio gain functions, if possible
    - fix a/v delay ring buffer offset and alert tone buffer slot
    - fix received audio gain

Original comment by jropal...@gmail.com on 10 Feb 2010 at 11:19

[GoogleCodeExporter]

- STREAM_MUSIC is used instead of STREAM_VOICE_CALL (see separate issue report 
on 
this subject).

- It's the check for clipping that does not work with your code (it's a short!).

- Noise generation is for speakerphone only.

The only "good part" of your changes is the change to VideoCamera at this time. 
Please further investigate in the audio gain problem. It should work with 
STREAM_MUSIC.

Original comment by pmerl...@googlemail.com on 11 Feb 2010 at 4:20

[GoogleCodeExporter]

OK, I've read issue 270 and also done a simple test, changing MUSIC to 
VOICE_CALL
throughout (in the code from trunk, not in my patched code).  I can report that 
the
Earpiece Gain settings DO work with VOICE_CALL on the Nexus One which is a 2.1 
device.

But I can also report that using the audgain() method in RtpStreamReceiver and
keeping MUSIC, as I did before, also solves the problem.  

Anyway, I will let you work through the discussion in issue 270 first, rather 
than
applying a separate patch here.

I note, while testing this, that the phone's volume switch changes the Ring 
Volume,
even when in a call.  It might make more sense to have the volume switch change 
the
Media Volume when a call is in progress?

As for the other issues...
   - I will fix my combining of calc/calc1/calc5/calc10 to use an int to test the
     clipping.  Thanks for catching that.
   - I think silence detection/comfort noise generation should always be done
     (speaker or handset).  In fact, when on a speakerphone, there is less likely
     to be total silence due to ambient room noise, but when on a handset silence
     can lead to "hello are you still there?"  I'd almost argue it is not needed
     if on speakerphone.
   - OK on the ring offset for the a/v sync.
I will work to separate out these issues into separate diffs and post them in
separate issue tickets for review prior to commiting.  This will probably take 
a few
days.

Original comment by jropal...@gmail.com on 11 Feb 2010 at 10:53

[GoogleCodeExporter]

When using STREAM_MUSIC in Sipdroid it should change media volume while in a 
call. 
Interesting that audio gain which uses track volume changes does not work in 
this 
case. Did you verify that?

Don't touch the calc routines please. An int would take more processing time. 
They 
have already undergone speed testing.

Noise is not generated for silence. This is used as an echo block for 
speakerphone.

I have already taken over your change of the ring offset for the next release.

Original comment by pmerl...@googlemail.com on 12 Feb 2010 at 9:22

[GoogleCodeExporter]

If I use your latest .apk, the phone's volume switch does change the Media 
Volume
when in a call.  But if I change the code to use VOICE_DATA instead of MUSIC, 
the
volume switch then changes the Ringer Volume when in a call.

I will abandon this whole patch for now.

But something is needed to make the handset/earpiece audio louder on the N1.  
It is
very hard to hear, especially when outdoors, even with the Media Volume all the 
way
up.  By comparison, calls on the GSM phone and also playing of music is very 
much
louder, and requires the media volume to be set to about 50% for a comfortable 
level.
 Adding the audgain() code to RtpStreamReceiver and then setting Earpiece Gain to
High or Highest in SIPdroid does fix this.

Do let me know if there's anything I can help with in testing alternative fixes 
or in
applying this one.

Original comment by jropal...@gmail.com on 13 Feb 2010 at 5:24

[GoogleCodeExporter]

What I asked you to verify is if you are sure there is no change on the N1 for 
different earpiece gains. This is not done by changing stream volume (as with 
the 
volume switches). It is done by changing the track volume. So if different 
earpiece 
gains don't result in different volumes heard in the earpiece there must be a 
bug in 
changing track volumes.

Original comment by pmerl...@googlemail.com on 14 Feb 2010 at 9:33

[GoogleCodeExporter]

Well, I believe it was not working at the time I started to look into this, 
else I
would not have embarked on it.

However, I loaded 1.3.14 from the market today and changing the Earpiece 
Setting does
indeed now change the volume in that version.

Not sure what code change fixed it, or if this was in fact due to other phone 
stuff
that is no longer relevant.

Original comment by jropal...@gmail.com on 16 Feb 2010 at 10:27

• Changed state: Done