What’s wrong with this code (COM Interop)?

A special “COM Interop” edition of “What’s Wrong with this code?”

This code is using some Windows Media interfaces to get the properties from a profile.

IWMMediaProps::GetMediaType() gets a _WMMediaType structure. Because there is optional format data that comes back, you have to call the method once to get the size, then again to get the actual data. You can assume that that extra data is a WAVEFORMATEX structure. In the C# world, the pbFormat field in _WMMediaType is an IntPtr.

Here’s the code. There’s something bad lurking there. I think I’ve given you enough information to figure it out.

IWMStreamConfig iStreamConfig = …;   // set somewhere else…
DirectShow.IWMMediaProps mediaProperties = (DirectShow.IWMMediaProps) iStreamConfig;

uint typeSize = 0;
mediaProperties.GetMediaType(null, ref typeSize);  // call to get size

byte[] buffer = new byte[typeSize];
mediaProperties.GetMediaType(buffer, ref typeSize); // call to fetch values

fixed (byte* pBuffer = buffer)
    mediaType = *((DirectShow._WMMediaType*) pBuffer);
    waveFormatEx = *((DirectShow.WAVEFORMATEX*) (mediaType.pbFormat.ToPointer()));


Comments (6)

  1. Daniel says:

    Just having a stab in the dark,

    the byte[] buffer, isn’t fixed, so it can be freely moved around? but the pointer pBuffer is fixed, but it’s a pointer, which points to an address where buffer might have been… So really the fixed section is all that useful in this case ?

  2. Nat says:

    Just a guess

    1. "fixed" block should be put before GetMediaType. Otherwise buffer may be moved around but buffer.pbFormat pointer will not be changed.

    2. instead of just cast the value to waveFormatEx, you might need to actually copy the value to use later since again after the fixed block, buffer may go out of scope and get garbage collected by GC. GC.KeepAlive might help but you would need to pin waveFormatEx.

  3. Another guess: pointer dereferencing doesn’t always work (though this may be a bug in Mono; I still need to test that under .NET for compatibility…).

    System.Runtime.InteropServices.Marshal.PtrToStructure should be used instead:

    // BAD

    mediaType = *((DirectShow._WMMediaType*) pBuffer);

    // GOOD

    mediaType = (DirectShow._WMMediaType) Marshal.PtrToStructure ((IntPtr) pBuffer, typeof (DirectShow._WMMediaType));

    waveFormatEx = (DirectShow.WAVEFORMATEX) Marshal.PtrToStructure (mediaType.pbFormat, typeof (DirectShow.WAVEFORMATEX));

    The key point is that the runtime needs to have a chance to marshal the unmanaged code into the managed structure (fixup alignment issues, if any, copy strings, etc.).

    At least, this was the case under Mono when I was trying to deal directly with a linked-list (in particular, with it’s ‘next’ pointer). Direct pointer dereference and assignment didn’t copy the pointer value; Marshal.PtrToStructure did. Again, that could be a Mono issue, but I wouldn’t be surprised if this was the issue here.

    I don’t think that there needs to be a "fixed" block around GetMediaType. The runtime marshalling system should automatically pin the buffer for you during the method call.

  4. Its in C++? All those *’s make me queesy.