Index: /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp =================================================================== --- /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp (revision 82342) +++ /trunk/src/VBox/Devices/Audio/DevIchAc97.cpp (revision 82343) @@ -143,5 +143,5 @@ /** @} */ -/* AC'97 uses 1.5dB steps, we use 0.375dB steps: 1 AC'97 step equals 4 PDM steps. */ +/** AC'97 uses 1.5dB steps, we use 0.375dB steps: 1 AC'97 step equals 4 PDM steps. */ #define AC97_DB_FACTOR 4 @@ -265,4 +265,6 @@ * Structures and Typedefs * *********************************************************************************************************************************/ +/** The ICH AC'97 (Intel) controller. */ +typedef struct AC97STATE *PAC97STATE; /** @@ -301,5 +303,5 @@ #ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO /** - * Structure keeping the AC'97 stream's state for asynchronous I/O. + * Asynchronous I/O state for an AC'97 stream. */ typedef struct AC97STREAMSTATEAIO @@ -318,12 +320,12 @@ volatile bool fEnabled; bool afPadding[5]; -} AC97STREAMSTATEAIO, *PAC97STREAMSTATEAIO; +} AC97STREAMSTATEAIO; +/** Pointer to the async I/O state for an AC'97 stream. */ +typedef AC97STREAMSTATEAIO *PAC97STREAMSTATEAIO; #endif -/** The ICH AC'97 (Intel) controller. */ -typedef struct AC97STATE *PAC97STATE; - -/** - * Structure for keeping the internal state of an AC'97 stream. + +/** + * The internal state of an AC'97 stream. */ typedef struct AC97STREAMSTATE @@ -367,7 +369,7 @@ /** - * Structure containing AC'97 stream debug stuff, configurable at runtime. - */ -typedef struct AC97STREAMDBGINFORT + * Runtime configurable debug stuff for an AC'97 stream. + */ +typedef struct AC97STREAMDEBUGRT { /** Whether debugging is enabled or not. */ @@ -382,17 +384,17 @@ * whereas for output streams this dumps data being read from the device DMA. */ R3PTRTYPE(PPDMAUDIOFILE) pFileDMA; -} AC97STREAMDBGINFORT, *PAC97STREAMDBGINFORT; - -/** - * Structure containing AC'97 stream debug information. - */ -typedef struct AC97STREAMDBGINFO -{ - /** Runtime debug info. */ - AC97STREAMDBGINFORT Runtime; -} AC97STREAMDBGINFO ,*PAC97STREAMDBGINFO; - -/** - * Structure for an AC'97 stream. +} AC97STREAMDEBUGRT; + +/** + * Debug stuff for an AC'97 stream. + */ +typedef struct AC97STREAMDEBUG +{ + /** Runtime debug stuff. */ + AC97STREAMDEBUGRT Runtime; +} AC97STREAMDEBUG; + +/** + * An AC'97 stream. */ typedef struct AC97STREAM @@ -410,7 +412,7 @@ uint32_t Padding1; #endif - /** Debug information. */ - AC97STREAMDBGINFO Dbg; -} AC97STREAM, *PAC97STREAM; + /** Debug stuff. */ + AC97STREAMDEBUG Dbg; +} AC97STREAM; AssertCompileSizeAlignment(AC97STREAM, 8); /** Pointer to an AC'97 stream (registers + state). */ @@ -420,5 +422,5 @@ #ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO /** - * Structure for the async I/O thread context. + * Asynchronous I/O thread context (arguments). */ typedef struct AC97STREAMTHREADCTX @@ -426,9 +428,12 @@ PAC97STATE pThis; PAC97STREAM pStream; -} AC97STREAMTHREADCTX, *PAC97STREAMTHREADCTX; +} AC97STREAMTHREADCTX; +/** Pointer to the context for an async I/O thread. */ +typedef AC97STREAMTHREADCTX *PAC97STREAMTHREADCTX; #endif /** - * Structure defining a (host backend) driver stream. + * A driver stream (host backend). + * * Each driver has its own instances of audio mixer streams, which then * can go into the same (or even different) audio mixer sinks. @@ -438,8 +443,10 @@ /** Associated mixer stream handle. */ R3PTRTYPE(PAUDMIXSTREAM) pMixStrm; -} AC97DRIVERSTREAM, *PAC97DRIVERSTREAM; - -/** - * Struct for maintaining a host backend driver. +} AC97DRIVERSTREAM; +/** Pointer to a driver stream. */ +typedef AC97DRIVERSTREAM *PAC97DRIVERSTREAM; + +/** + * A host backend driver (LUN). */ typedef struct AC97DRIVER @@ -468,7 +475,12 @@ /** Driver stream for output. */ AC97DRIVERSTREAM Out; -} AC97DRIVER, *PAC97DRIVER; - -typedef struct AC97STATEDBGINFO +} AC97DRIVER; +/** Pointer to a host backend driver (LUN). */ +typedef AC97DRIVER *PAC97DRIVER; + +/** + * Debug settings. + */ +typedef struct AC97STATEDEBUG { /** Whether debugging is enabled or not. */ @@ -476,9 +488,10 @@ /** Path where to dump the debug output to. * Defaults to VBOX_AUDIO_DEBUG_DUMP_PCM_DATA_PATH. */ - char szOutPath[RTPATH_MAX + 1]; -} AC97STATEDBGINFO, *PAC97STATEDBGINFO; - -/** - * Structure for maintaining an AC'97 device state. + char szOutPath[RTPATH_MAX]; +} AC97STATEDEBUG; + + +/** + * The shared AC'97 device state. */ typedef struct AC97STATE @@ -524,5 +537,6 @@ /** The base interface for LUN\#0. */ PDMIBASE IBase; - AC97STATEDBGINFO Dbg; + /** Debug settings. */ + AC97STATEDEBUG Dbg; /** PCI region \#0: NAM I/O ports. */ @@ -540,6 +554,5 @@ } AC97STATE; AssertCompileMemberAlignment(AC97STATE, aStreams, 8); -/** Pointer to a AC'97 state. */ -typedef AC97STATE *PAC97STATE; + /** @@ -633,5 +646,4 @@ # ifdef VBOX_WITH_AUDIO_AC97_ASYNC_IO -static DECLCALLBACK(int) ichac97R3StreamAsyncIOThread(RTTHREAD hThreadSelf, void *pvUser); static int ichac97R3StreamAsyncIOCreate(PAC97STATE pThis, PAC97STREAM pStream); static int ichac97R3StreamAsyncIODestroy(PAC97STATE pThis, PAC97STREAM pStream); @@ -952,5 +964,5 @@ RTStrPrintf(szFile, sizeof(szFile), "ac97StreamReadSD%RU8", pStream->u8SD); - char szPath[RTPATH_MAX + 1]; + char szPath[RTPATH_MAX]; int rc2 = DrvAudioHlpFileNameGet(szPath, sizeof(szPath), pThis->Dbg.szOutPath, szFile, 0 /* uInst */, PDMAUDIOFILETYPE_WAV, PDMAUDIOFILENAME_FLAGS_NONE); @@ -1207,4 +1219,8 @@ RTThreadUserSignal(hThreadSelf); + /** @todo r=bird: What wasn't mentioned by the original author of this + * code, is that pCtx is now invalid as it must be assumed to be out + * of scope in the parent thread. It is a 'ing stack object! */ + LogFunc(("[SD%RU8] Started\n", pStream->u8SD)); @@ -1269,5 +1285,19 @@ if (RT_SUCCESS(rc)) { +/** @todo r=bird: Why is Ctx on the stack? There is no mention of this in + * the thread structure. Besides, you only wait 10seconds, if the + * host is totally overloaded, it may go out of scope before the new + * thread has finished with it and it will like crash and burn. + * + * Also, there is RTThreadCreateF for giving threads complicated + * names. + * + * Why aren't this code using the PDM threads (PDMDevHlpThreadCreate)? + * They would help you with managing stuff like VM suspending, resuming + * and powering off. + * + * Finally, just create the threads at construction time. */ AC97STREAMTHREADCTX Ctx = { pThis, pStream }; +# error "Busted code! Do not pass a structure living on the parent stack to the poor thread!" char szThreadName[64];