[jitsi-dev] Some coding issues / questions


#1

Dear all,

while debugging some audio problems I saw some lines of code that
may be wrond/problematic:

PortAudioRenderer/process(...) - audio level processing loop
at line ~520: this modifies the buffer but does not take the offest
into the buffer into account (always starts at "i = 0"). Ususally
the offset is zero thus it works. However, this may change :slight_smile: .

net_java_sip_communicator_impl_neomedia_portaudio_PortAudio.c,
method ....PortAudio_Pa_1WriteStream(....).

The following code snippet is strange:
...
    for (i = 0; i < numberOfWrites; i++)
    {
        errorCode = Pa_WriteStream(paStream, data, frames);
        if ((paNoError != errorCode) && (errorCode != paOutputUnderflowed))
            break;
        else
        {
            if (audioQualityImprovement)
            {
                AudioQualityImprovement_process(
                    audioQualityImprovement,
                    AUDIO_QUALITY_IMPROVEMENT_SAMPLE_ORIGIN_OUTPUT,
                    sampleRate,
                    sampleSizeInBits,
                    channels,
                    data,
                    framesInBytes);
            }
            data += framesInBytes;
        }
    }
...

Have a look at the error check just before the break statement: Usually
Pa_WriteStream returns paNoError (rarely Underflowed): thus this "if" check
evaluates to false and AudioQualityImprovement is done, but in this case
for data that was already handed over to portaudio - thus "after the fact".

Is this correct? What is the real proposed behaviour for this code?
AudioImprovment should be done before Pa_WriteStream()?

Regards,
Werner


#2

PortAudioRenderer/process(...) - audio level processing loop
at line ~520: this modifies the buffer but does not take the offest
into the buffer into account (always starts at "i = 0"). Ususally
the offset is zero thus it works. However, this may change :slight_smile: .

Werner, you're right. This is new code that I haven't reviewed. As can
be seen at line ~533, the old code is already taking offset into
account so we should definitely be caring about it elsewhere.

Have a look at the error check just before the break statement: Usually
Pa_WriteStream returns paNoError (rarely Underflowed): thus this "if" check
evaluates to false and AudioQualityImprovement is done, but in this case
for data that was already handed over to portaudio - thus "after the fact".

Is this correct? What is the real proposed behaviour for this code?
AudioImprovment should be done before Pa_WriteStream()?

AudioQualityImprovement does not improve output quality, it only tries
to improve the input quality. And since echo cancellation tries to
cancel echo from the input, it takes into account "output" that has
already been written. That's why AudioQualityImprovement receives the
output "after the fact". Otherwise, echo cancellation may end up being
asked to cancel echo based on "output" that hasn't even been given to
PortAudio for writing.

···

On Wed, Mar 2, 2011 at 7:47 PM, Werner Dittmann <Werner.Dittmann@t-online.de> wrote: