LMMS / lmms

Cross-platform music production software
https://lmms.io
GNU General Public License v2.0
7.8k stars 986 forks source link

Buggy dead code in AudioDevice.cpp and not so dead in AudioJack.cpp #6072

Open firewall1110 opened 3 years ago

firewall1110 commented 3 years ago

Bug Summary

AudioDevice.cpp frome line 185 (code with my comments):

fpp_t AudioDevice::resample( const surroundSampleFrame * _src,
                        const fpp_t _frames,
                        surroundSampleFrame * _dst,
                        const sample_rate_t _src_sr,
                        const sample_rate_t _dst_sr )
{
    if( m_srcState == NULL )
    {
        return _frames;
    }
    m_srcData.input_frames = _frames;
    m_srcData.output_frames = _frames;
    m_srcData.data_in = const_cast<float*>(_src[0].data());
    m_srcData.data_out = _dst[0].data ();
    m_srcData.src_ratio = (double) _dst_sr / _src_sr;
    m_srcData.end_of_input = 0;
    int error;
    if( ( error = src_process( m_srcState, &m_srcData ) ) )
    {
        printf( "AudioDevice::resample(): error while resampling: %s\n",
                            src_strerror( error ) );
    }
    // m_srcDate is used only in AudioDevice::resample
    // m_srcDate.input_frames_used            is not used :
    //    when  src_process(...) use less frames than given
    //    some frames will be skipped with sound corruption
    return static_cast<fpp_t>(m_srcData.output_frames_gen);
}

AudioJack.cpp line 388:

    jack_nframes_t done = 0;
    while( done < _nframes && m_stopped == false )
    {
        // If we are here, than m_stopped == false 
        // if resampling is not used, 
        jack_nframes_t todo = qMin<jack_nframes_t>(
                        _nframes,
                        m_framesToDoInCurBuf -
                            m_framesDoneInCurBuf ); 
                            // this is the same as (todo = _nframes;)
        // That's why  code is working fine  ... (if resampling is not used!)
        const float gain = mixer()->masterGain();
        for( int c = 0; c < channels(); ++c )
        {
            jack_default_audio_sample_t * o = m_tempOutBufs[c];
            for( jack_nframes_t frame = 0; frame < todo; ++frame )
            {
                o[done+frame] = m_outBuf[m_framesDoneInCurBuf+frame][c] * gain;
            }
        }
        done += todo;
        m_framesDoneInCurBuf += todo;
        if( m_framesDoneInCurBuf == m_framesToDoInCurBuf )
        {
            // when using resampling getNextBuffer( ... )
            // can return any value (in some range) 
            m_framesToDoInCurBuf = getNextBuffer( m_outBuf ); // will stopped if return 0
            m_framesDoneInCurBuf = 0;
            if( !m_framesToDoInCurBuf )
            {
                m_stopped = true; // only if getNextBuffer( ) returns 0 : happens rarely
                break; // so here while  can be exited with (done < _nframes) == true
            }
        }
    }
    // we see that, it is highly likely (done >= _nframes) 
    // and variant with (done >   _nframes )  is if resampling is using  
    if( _nframes != done ) 
    // if ( _nframes > done ) :: is not a solution (only SEGFAULT workaround):
    // buffer overflow is happening before, and some samples are skipped - sound corruption
    {
        // What happens if (done >   _nframes ) in some small value ? 
        // [next comments this situation]
        for( int c = 0; c < channels(); ++c )
        {
            jack_default_audio_sample_t * b = m_tempOutBufs[c] + done; // out off buffer
            memset( b, 0, sizeof( *b ) * ( _nframes - done ) ); // memset to ~ huge size : SEGFAULT
        }
    }

    return 0;
}

Steps to reproduce

Can not be reproduced: now sampling rates and buffer sizes in AudioDevice and Mixer are the same. Behavior is from some experiments out off LMMS context

Expected behavior

clean sound without segmentation faults

Actual behavior

segmentation fault in AudioJack.cpp memset( b, 0, sizeof( *b ) * ( _nframes - done ) ); corrupted sound after work around the SEGFAULT whith: if ( _nframes > done )

Affected LMMS versions

current LMMS master

Rossmaxx commented 1 month ago

I believe @sakertooth removed this in one of his PRs related to audio devices.