My second patch …

(February 7, 2006)

I just submitted my second patch to a public open-source software project. The first one was about one or two years ago, a new feature for a not-too-famous image viewer named QIV. The project I submitted today’s patch to is, erm, somewhat bigger: it’s MPlayer. It’s a patch for the DirectX video output driver that used to take 100% CPU time on some (but not all) systems.

I already found the bug a few days ago: The problem is that Directx_ManageDisplay() in vo_directx.h is called for every frame if MPlayer is embedded into another window (using the –wid option). This function performs some amount of initialization: It queries DirectX capabilities and sets up some other DirectX stuff. The rationale behind calling this function so frequently is quite clear to me: In embedded mode, MPlayer doesn’t receive window move and resize notifications from Windows. Thus, the developer decided to simply call the function frequently enough to keep track of the current window position and size. What he didn’t know is that some Windows, DirectX or graphics driver versions don’t like this and occupy the CPU.

To test the modifications I was going to do, I needed a MPlayer build environment first. This involved setting up a MinGW installation on my box, compiling about a dozen libraries MPlayer required for extended functionality and finally building MPlayer itself. I’m absolutely sure I would have furiously canceled my attempts to do all that for myself after half an hour or so – the relation between MinGW and MSYS alone is just too complicated to get it. But fortunately, there is a really excellent step-by-step manual about how to do it. With this handy document, building MPlayer was a walk in the park. The only thing that could pose a problem to other users is the »check out x264 from Subversion somehow« part; but since I have a x264 checkout on my Linux box, I simply copied it over via scp.

The fix essentially does nothing but »window position caching«: Right at the beginning of Directx_ManageDisplay(), the window’s extent is queried by GetWindowRect(), which is a trivial Win32 API call that executes in virually no time. The resulting position and size is compared to the values from the last frame. If the window is at excactly the same place as it used to be those ~40 ms ago, all the lengthy initialization is skipped. This simple change (16 lines, 2 of which are additional »safety nets«) reduces MPlayer’s CPU load to reasonable values again. I can just hope that this doesn’t introduce any new bugs …

7 Responses to »My second patch …«

Post a Reply

XHTML: You can use these tags: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>


By submitting the comment, you agree to the terms of the Privacy Policy.