Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
firewall1110 opened this issue Jul 2, 2021 · 1 comment
Open
Labels

Comments

@firewall1110
Copy link
Contributor

firewall1110 commented Jul 2, 2021

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
Copy link
Contributor

Rossmaxx commented May 9, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants