XAPO BaseProcessor - incorrect registration properties handling

[GoogleCodeExporter]

Hello,

I wrote a custom equalizer XAPO using SlimDX, and experienced a strange 
behavior with it. Sometimes it worked well, but often it wasn't possible to 
play any sound, and the debug version of XAudio2 engine reported strange issues 
in SubmixVoice.SetEffectChain() call, like this:

XAUDIO2: ERROR: Effect 0: cannot work with a single input buffer 
XAUDIO2: ERROR: Effect 0: cannot work with a single output buffer 
XAUDIO2: ERROR: Invalid pEffectChain argument 

After some investigation I found out, that my XAPO enters the SetEffectChain() 
method with severely damaged registration properties. The cause of this problem 
seems to be in the handling of "props" variable in BaseProcessor constructor:

BaseProcessor::BaseProcessor( SlimDX::XAPO::RegistrationProperties properties )
{
    XAPO_REGISTRATION_PROPERTIES props = properties.ToUnmanaged();

    Construct( new XAPOBaseImpl( this, props ) );
}

The problem is, that "props" is just a local variable, it doesn't survive past 
the end of the constructor scope. Therefore, at the moment of SetEffectChain() 
call, which comes at a later time, the pointer to the "props" structure may 
already "look" at some random data, that could be written in that location 
anytime between the constructor and SetEffectChain() calls.

Making the "props" a member variable of the BaseProcessor class, instead of 
local variable, and changing the constructor in following way:

BaseProcessor::BaseProcessor( SlimDX::XAPO::RegistrationProperties properties )
{
    props = new XAPO_REGISTRATION_PROPERTIES();

    *props = properties.ToUnmanaged();

    Construct( new XAPOBaseImpl( this, props ) );
}

seems to fix the problem entirely.

Original issue reported on code.google.com by david.sv...@gmail.com on 7 Dec 2010 at 3:26

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Your solution likely introduces a memory leak.

CXAPOBase, which is the ultimate parent of XAPOBaseImpl, takes 
XAPO_REGISTRATION_PROPERTIES* but it's not clear if it expects those properties 
to live beyond the constructor invocation -- the documentation doesn't say.

If it does, we should store a copy of the properties in the XAPOBaseImpl 
object. If it doesn't, the bug is probably elsewhere.

Original comment by josh.petrie on 7 Dec 2010 at 4:38

[GoogleCodeExporter]

Mike presumably knows more about this, I've never done anything with this API.

Original comment by josh.petrie on 7 Dec 2010 at 4:40

[GoogleCodeExporter]

Hello Josh,

Thank you for a quick response... 

I didn't meant this to be a final solution - I am a complete newbie in SlimDX, 
so I know nothing about it's internal implementation concepts, and I am even 
not very skilled in C++ programming, so I published this more as the proof of 
my theory about the problem with local variable. 

It fixed the XAPO_REGISTRATION_PROPERTIES corruption in my case - and I think 
this is also the answer to your question, whether the properties are expected 
to live beyond the constructor: I am quite sure they must live beyond, because 
the XAPO is being asked about them during SetFilterChain() call...

Original comment by david.sv...@gmail.com on 7 Dec 2010 at 6:29

[GoogleCodeExporter]

This issue was closed by revision r1793.

Original comment by Mike.Popoloski on 11 Dec 2010 at 8:25

• Changed state: Fixed

[GoogleCodeExporter]

I made the fix. Let me know if it resolved your issue.

Original comment by Mike.Popoloski on 11 Dec 2010 at 8:26

[GoogleCodeExporter]

Hi Mike,

I've tried it, and it seems to behave well, now... Thanks a lot!

David

Original comment by david.sv...@gmail.com on 15 Dec 2010 at 6:30

[GoogleCodeExporter]

Glad to hear it.

Original comment by Mike.Popoloski on 15 Dec 2010 at 6:41