Playing Audio CDs, part 9 - Fixing Glitches

  • Article

When we last left playing back audio, we had playback working, but it was glitching like CRAZY - literally every packet read had a glitch during playback.

So lets see if we can fix the glitching problem.

As I mentioned yesterday, the root cause of the glitches is that we're waiting on the wave playback to complete - we're doing all our I/O through a single buffer.

So what happens if you don't wait for the wave writes to complete?  It's a simple change to make, maybe it will help with playback.

 

HRESULT CDAENoWaitPlayer::PlayTrack(int TrackNumber){    HRESULT hr;    HANDLE waveWriteEvent = CreateEvent(NULL, FALSE, FALSE, NULL);    HWAVEOUT waveHandle = OpenWaveForCDAudio(waveWriteEvent);    if (waveHandle == NULL)    {        return E_FAIL;    }    TrackNumber -= 1; // Bias the track number by 1 - the track array is )ORIGIN 0.    CAtlList<CDRomReadData *> readDataList;    for (DWORD i = 0 ; i < (_TrackList[TrackNumber]._TrackLength / DEF_SECTORS_PER_READ); i += 1)    {        CDRomReadData *readData = new CDRomReadData(DEF_SECTORS_PER_READ);        if (readData == NULL)        {            printf("Failed to allocate a block\n");            return E_FAIL;        }        readData->_RawReadInfo.DiskOffset.QuadPart = ((i * DEF_SECTORS_PER_READ) + _TrackList[TrackNumber]._TrackStartAddress)*                                                      CDROM_COOKED_BYTES_PER_SECTOR;        readData->_RawReadInfo.TrackMode = CDDA;        readData->_RawReadInfo.SectorCount = DEF_SECTORS_PER_READ;        hr = CDRomIoctl(IOCTL_CDROM_RAW_READ, &readData->_RawReadInfo, sizeof(readData->_RawReadInfo), readData->_CDRomData,                             readData->_CDRomDataLength);         if (hr != S_OK)        {            printf("Failed to read CD Data: %d", hr);            return hr;        }        MMRESULT waveResult;        readData->_WaveHdr.dwBufferLength = readData->_CDRomAudioLength;        readData->_WaveHdr.lpData = (LPSTR)readData->_CDRomData;        readData->_WaveHdr.dwLoops = 0;        waveResult = waveOutPrepareHeader(waveHandle, &readData->_WaveHdr, sizeof(readData->_WaveHdr));        if (waveResult != MMSYSERR_NOERROR)        {            printf("Failed to prepare wave header: %d", waveResult);            return HRESULT_FROM_WIN32(waveResult);        }        waveResult = waveOutWrite(waveHandle, &readData->_WaveHdr, sizeof(readData->_WaveHdr));        if (waveResult != MMSYSERR_NOERROR)        {            printf("Failed to write wave header: %d", waveResult);            return HRESULT_FROM_WIN32(waveResult);        }        //        // Add this buffer to the end of the read queue.        //        readDataList.AddTail(readData);        //        // See if the block at the head of the list is done. If it is, free it and all the other completed        // blocks at the head of the list.        //        // Because we know that the wave APIs complete their data in order, we know that the first buffers in the list        // will complete before the last - the list will effectively be sorted by completed state        //        while (!readDataList.IsEmpty() && readDataList.GetHead()->_WaveHdr.dwFlags & WHDR_DONE)        {            CDRomReadData *completedBlock;            completedBlock = readDataList.RemoveHead();            waveOutUnprepareHeader(waveHandle, &completedBlock->_WaveHdr, sizeof(completedBlock->_WaveHdr));            delete completedBlock;        };    }    //    //    We're done, return    //    return S_OK;}

So what changed?  First off, instead of allocating a single block, we allocate a CDRomReadData object for every single read.  We take advantage of the fact that the wave APIs queue their writes and simply drop the request into the waveOutWrite API when the data's been read.

Since the blocks of data are much smaller than the length of the track, the odds are high that we'll be done with some of the blocks before we finish reading the audio track, so we use the fact that the wave header has a flag that indicates that the wave writer is done with a block to let us know when it's ok to free up the block.

So when I tested this version of the playback, the glitches were gone (good!).  But the playback stopped after a relatively short time - certainly before the end of the track (I was using Ravel's Bolero as my test case - the clarinet solo at the beginning of the crescendo is a great test to listen for glitches).  But Bolero's about 15 minutes long, and the playback was finishing up after about 1 minute or so.

Why?  Because my CDROM drive is faster than the audio playback - the audio data had been read and queued for the entire track, but it hadn't finished playing back.  If you think about it, reading the data was done at 48x (or whatever the speed of the CDROM drive is), but the playback is done at 1x.

So we need to add one more piece to the routine - just before we return S_OK, we need to put the following loop:

    //    // Now drain the requests in the queue.    //    while (!readDataList.IsEmpty())    {        if (readDataList.GetHead()->_WaveHdr.dwFlags & WHDR_DONE)        {            CDRomReadData *completedBlock;            completedBlock = readDataList.RemoveHead();            waveOutUnprepareHeader(waveHandle, &completedBlock->_WaveHdr, sizeof(completedBlock->_WaveHdr));            delete completedBlock;        }        else        {                Sleep(100);        }    };

This loop waits until all the queued reads complete (and frees them).

But there's still a problem with the code - on my machine, playing Bolero, there were 4056 CDRomReadData objects on the readDataList queue that had to be freed during the final rundown list.  The PlayCDWMP application took up 156M of memory.  Essentially we'd read all the audio data into memory during the read process.  Not good.  Note that we didn't LEAK the memory (I fixed that problem) - we know where every bit of the memory is.  But we've allocated WAY more than we should have.

Next time, lets see if we can work on fixing the memory management problem.

 

Edit: Raymond gently reminded me :) that Sleep(0) throws the system into a CPU bound loop, so changed the sleep to actually sleep for some time.

Edit2: For those that complained, added the waveOutUnprepareHeader call.