Monday, November 27, 2006

SetSurfaceDesc memory leak

The CVideo object is leaking memory badly, and I'm surprised I never noticed it before. It leaks the entire DirectSurface every time a new video is opened. At 640 x 480 that's around 1 MB per open, which adds up fast. This could explain why Whorld misbehaves after many hours of triggering video clips.

Initially I suspected VfW, but then I verified that VfW definitely cleans up after itself. The problem turns out to be with DirectDraw, specifically with SetSurfaceDesc.

CVideo's constructor creates a default 1 x 1 memory surface. When a video is opened, CVideo attaches this surface to the video frame, using SetSurfaceDesc. This is a major optimization: it allows CVideo to avoid using GDI to blit each video frame to the DirectDraw surface, because the video frame IS the DirectDraw surface. In practice SetSurfaceDesc only needs to be called once, when the video opens, because VfW doesn't change the address of video frame after that. In fact it only changes the address if you open a video with a different frame size or pixel format, sensibly enough. CVideo checks for a change in frame buffer address, and if one occurs, it reattaches its surface to the new address.

According to the MSDN on SetSurface, "The DirectDrawSurface object will not deallocate surface memory that it didn't allocate. Therefore, when the surface memory is no longer needed, it is your responsibility to deallocate it. However, when SetSurfaceDesc is called, DirectDraw frees the original surface memory that it implicitly allocated when creating the surface."

I interpreted this to mean that once you've done at least one SetSurfaceDesc for a surface, you're on your own, as far as memory management. But what happens is, when the video is closed, DirectDraw leaves some object the same size as the frame buffer object allocated. It can't be VfW's frame buffer, because VfW destroys that when you call AVIStreamGetFrameClose. I can't imagine how or why this happens, but it sure isn't documented.

I only found two ways to make DirectDraw release this mysterious hidden frame buffer. The obvious way is to destroy the surface, but I'd prefer not to do this, because it means destroying and re-creating the surface every time a video is opened, which seems wasteful. The other way is to call SetSurfaceDesc again, passing it a 1 x 1 dummy surface. This works fine, and only takes about 50 microseconds. The surface description never changes, so it can even be a static array.

DDSURFACEDESC CVideo::m_DefSurf = {
sizeof(DDSURFACEDESC), // dwSize
DDSD_WIDTH | DDSD_HEIGHT | DDSD_PITCH | DDSD_LPSURFACE | DDSD_PIXELFORMAT, // dwFlags
1, // dwHeight
1, // dwWidth
4, // lPitch (Width * BitCount / 8)
0, 0, 0, 0, // dwBackBufferCount, dwMipMapCount, dwAlphaBitDepth, dwReserved
&m_DefSurfMem, // lpSurface
{0, 0}, {0, 0}, {0, 0}, {0, 0}, // color keys
{
sizeof(DDPIXELFORMAT), // dwSize
DDPF_RGB, // dwFlags
0, // dwFourCC
32, // dwRGBBitCount
0xff0000, // dwRBitMask
0x00ff00, // dwGBitMask
0x0000ff // dwBBitMask
}
};
DWORD CVideo::m_DefSurfMem; // pointed to by m_DefSurf.lpSurface
...
void CVideo::Close()
{
// if surface exists, we must attach it to a default 1 x 1 memory surface,
// otherwise DirectDraw leaves a mysterious hidden frame buffer allocated
if (m_Surface != NULL)
m_Surface->SetSurfaceDesc(&m_DefSurf, 0); // prevents a major leak
...

No comments: