Larry Osterman's WebLog

Confessions of an Old Fogey
Blog - Title

Playsound is failing on Vista! What's wrong?

Playsound is failing on Vista! What's wrong?

  • Comments 23

Recently BillP, the author of the antispyware application WinPatrol asked on the MSDN forums about a problem he was having with his application.

His app called PlaySound(MAKEINTRESOURCE(IDR_WOOF), hInst, SND_RESOURCE | SND_SYNC | SND_NOWAIT) but it was failing (returning false).

He was wondering if this might be a bug in Vista's playsound implementation - a reasonable assumption given that his application worked just fine on previous versions of Windows.

 

I knew that we were passing all the PlaySound regression tests, and there are a number of elements of Windows that use SND_RESOURCE (the pearl startup sound is one of them), so the SND_RESOURCE functionality wasn't broken.  I was puzzled for a bit and then I realized that there WAS one change to PlaySound in Vista (other than some general code cleanup, the addition of support for the SND_SENTRY and SND_SYSTEM flags and support for accessibility events on PlaySound).

For Vista, I tightened up the validation logic that's used when checking files before the PlaySound call.  Among other things, we check to make sure that:

a) The cbSize field in the "fmt " tag in the WAV file is less than 1K in length and
b) The WAVEFORMATEX in the "fmt " tag in the WAV file fits inside the "fmt " tag (done by checking that the waveformatex->cbSize+sizeof(WAVEFORMATEX) is less than the size of the "fmt " chunk[1]).

I downloaded BillP's app, and checked the resources in the file.  And in sure enough, the WAVEFORMATEX in the file had a length of 0x38 when it should have been 0.  Once I patched his binary to change the 0x38 to a 0, his application stared barking away.

At this point there are two options:  (1) BillP can fix his application to correct the corrupted resource or (2) Microsoft can change the PlaySound API to allow this kind of corruption (either to allow a bogus cbSize or to edit the app's WAVEFORMATEX to "fix" the bogus cbSize).  Changing PlaySound is not trivial at this point, and it's not clear what the right fix is - the error might be in the size of the "fmt " chunk, which means that the information in the cbSize field might be accurate.  In addition, the downstream audio rendering APIs are likely to choke on these malformed structures anyway. 

 

It's this kind of subtle breaking change that makes modifying any of the older Windows APIs such a nightmare. 

 

[1] We only check if the "fmt " chunk size is greater than sizeof(WAVEFORMAT) - if the "fmt " chunk is sizeof(WAVEFORMAT) than we assume that this structure is a WAVEFORMAT structure, which doesn't have a cbSize field. 

  • BillP was NOT a lazy programmer.  His app had a bug, and it was a very subtle one.

  • Lazy how?  Bill likely didn't know what the WAVEFORMATEX had in it; likely just asking Microsoft Visual C++ to simply add a wav file to the projects resources.

    How is that being a "Lazy Programmer".

  • Last sentence in the blog post reads "...than we assume...". I have many, many times seen the use of "then" where it should be "than". This is possibly the first I see the opposite.

    (English isn't even my first language) :-)

  • It doesn't sound like BillP hand-crafted the wav file and its header so I don't see how people can blame him for this. He had the misfortune to use a program which creates wav files with a bad header that happen to play in just about everything until now.

    That kind of issue isn't his fault (except perhaps for choosing the wrong software, but if it worked fine apart from this, and there was no way until now to know this issue existed, then I don't think you can say it was a poor choice) and is bloody hard to track down without access to the source code that is playing it (which is where Larry stepped in).

    You'd have to realise that some wav files play and others don't, then try to work out why, then have to learn about the wav file format (and BillP isn't writing a music app so why should he know about that?), then guess what it was that Vista was rejecting.

    People cry "lazy programmer" far too often. In some cases, like this, without understanding the problem. Even in other cases, programmers are rarely lazy; they just have a million other things to do and I think more problems are due to that, plus ignorance or lack of design skills, than actual laziness.

  • So, over in Raymond's suggestion box is a question that asks, "Is the checked build useful..." As we have all been told, the checked build has additional API argument checking and additional informational messages output to the debugger. PlaySound does not set GetLastError(), but would it have output a message to the debugger explaining the failior, or at least hinting at it?

  • In my last post , I enumerated a bewildering array of threats that the PlaySound API is subject to, today

  • It's been a long path, but we're finally at the point where I can finally present the threat model for

  • It's been a long path, but we're finally at the point where I can finally present the threat

Page 2 of 2 (23 items) 12