Re: [fluid-dev] Subversion checkins

[Rui Nuno Capela]

On Tue, September 18, 2007 06:12, Josh Green wrote:
> On Mon, 2007-09-17 at 23:42 -0500, David Hilvert wrote:
>
>> On Mon, 17 Sep 2007 23:38:24 -0500
>> David Hilvert <address@hidden> wrote:
>>
>>
>>> ==26661== Thread 2:
>>> ==26661== Invalid read of size 4
>>> ==26661==    at 0x4078F14: fluid_synth_dither_s16 (fluid_synth.c:1824)
>>>  ==26661==    by 0x4056651: fluid_alsa_audio_run_s16
>>> (fluid_alsa.c:558)
>>> ==26661==    by 0x40F42D2: start_thread (in /lib/libpthread-2.6.1.so)
>>> ==26661==    by 0x4BFE2FD: clone (in /lib/libc-2.6.1.so)
>>> ==26661==  Address 0x5571C88 is not stack'd, malloc'd or (recently)
>>> free'd ==26661==
>>> ==26661== Invalid read of size 4
>>> ==26661==    at 0x4078F4B: fluid_synth_dither_s16 (fluid_synth.c:1828)
>>>  ==26661==    by 0x4056651: fluid_alsa_audio_run_s16
>>> (fluid_alsa.c:558)
>>> ==26661==    by 0x40F42D2: start_thread (in /lib/libpthread-2.6.1.so)
>>> ==26661==    by 0x4BFE2FD: clone (in /lib/libc-2.6.1.so)
>>> ==26661==  Address 0xC4BD8AE0 is not stack'd, malloc'd or (recently)
>>> free'd
>>
>> It seems as though, rather than dither_index being trampled on, the
>> synth pointer might be bad.  Is the pointer value (and its
>> initialization, etc.) thread-safe?
>>
>
> I just discovered what is wrong.  Mystery is, I don't see how it would
> work at all on any system with meters enabled in QSynth.
>
> Seems some of the audio driver re-working I did caused the problem.  The
> new_fluid_audio_driver2 does not require a synth instance in order to
> function, since the callback function is responsible for rendering the
> audio.  Somehow I overlooked this fact and assumed that the data parameter
> was the Synth, when in the case of QSynth its actually its own internal
> synth class.
>
> I'll have to think more of how to fix this problem though, since now
> that the 16 bit dithering is used, there are some variables in the synth
> instance that need to be accessed even in the case of the audio callback
> method.  Probably won't be until later this week when I can get to it.
>
> Thanks again for all the probing!
>

yeah, good show. indeed new_fluid_audio_driver2() only takes a opaque
argument that is feeded unchanged to the callback function, and in case of
qsynth, that pointer is of a qsynthEngine instance and _not_ a
fluid_synth_t as you (wrongly;) assumed.

after all and out of curiosity, the crash symptoms were just about data
alignment as i have suspected in the first place ;) as the very first data
member of qsynthEnngine class is in effect the respective fluid_synth_t*
one. that almost explains the intermitent crash behavior: it just varies 
how the qtractorEngine instance is being loaded in memory and that could
actually change with the working set, compiler, linker-loader whatever.
but sometimes it can get it right just by coincidence ;)

conclusion: it does seem that the new_fluid_audio_driver2() function
should be considered deprecated and a new one should follow, eg.
new_fluid_audio_driver3() which would include an additional but mandatory
parameter in its signature as for the old forgotten fluid_synth_t*
instance.

qsynth can try to use this new api entry whenever available at ./configure
time.

cheers
-- 
rncbc aka Rui Nuno Capela
address@hidden