Changeset 95645 in vbox

Timestamp:

Jul 14, 2022 10:33:14 AM (2 years ago)

Author:

vboxsync

Message:

Recording/Main: Simplified and got rid of some destruction races by not dynamically (re-)allocating the recording context. Added docs, a bit of renaming. bugref:9286

Location:

trunk/src/VBox/Main

Files:

• include/ConsoleImpl.h (modified) (2 diffs)
• include/Recording.h (modified) (3 diffs)
• src-client/ConsoleImpl.cpp (modified) (9 diffs)
• src-client/Recording.cpp (modified) (6 diffs)

trunk/src/VBox/Main/include/ConsoleImpl.h

@@ -174 +174 @@
     AudioVideoRec *i_recordingGetAudioDrv(void) const { return mRecording.mAudioRec; }
 # endif
-    RecordingContext *i_recordingGetContext(void) const { return mRecording.mpCtx; }
+    RecordingContext *i_recordingGetContext(void) { return &mRecording.mCtx; }
 # ifdef VBOX_WITH_AUDIO_RECORDING
     HRESULT i_recordingSendAudio(const void *pvData, size_t cbData, uint64_t uDurationMs);
@@ -1156 +1156 @@
     {
         Recording()
-            : mpCtx(NULL)
 # ifdef VBOX_WITH_AUDIO_RECORDING
-            , mAudioRec(NULL)
+            : mAudioRec(NULL)
 # endif
         { }
 
         /** The recording context. */
-        RecordingContext     *mpCtx;
+        RecordingContext      mCtx;
 # ifdef VBOX_WITH_AUDIO_RECORDING
         /** Pointer to capturing audio backend. */

trunk/src/VBox/Main/include/Recording.h

@@ -36 +36 @@
 public:
 
-    RecordingContext(Console *pConsole, const settings::RecordingSettings &a_Settings);
+    RecordingContext();
+
+    RecordingContext(Console *ptrConsole, const settings::RecordingSettings &settings);
 
     virtual ~RecordingContext(void);
@@ -46 +48 @@
     size_t GetStreamCount(void) const;
 
-    int Create(const settings::RecordingSettings &a_Settings);
+    int Create(Console *pConsole, const settings::RecordingSettings &settings);
     void Destroy(void);
 
@@ -70 +72 @@
 protected:
 
-    int createInternal(const settings::RecordingSettings &a_Settings);
+    int createInternal(Console *ptrConsole, const settings::RecordingSettings &settings);
     int startInternal(void);
     int stopInternal(void);

trunk/src/VBox/Main/src-client/ConsoleImpl.cpp

@@ -5887 +5887 @@
     if (pDisplay)
     {
-        const bool fIsEnabled =    mRecording.mpCtx
-                                && mRecording.mpCtx->IsStarted();
+        bool const fIsEnabled = mRecording.mCtx.IsStarted();
 
         if (RT_BOOL(fEnable) != fIsEnabled)
@@ -5904 +5903 @@
 # ifdef VBOX_WITH_AUDIO_RECORDING
                         /* Attach the video recording audio driver if required. */
-                        if (   mRecording.mpCtx->IsFeatureEnabled(RecordingFeature_Audio)
+                        if (   mRecording.mCtx.IsFeatureEnabled(RecordingFeature_Audio)
                             && mRecording.mAudioRec)
                         {
-                            vrc = mRecording.mAudioRec->applyConfiguration(mRecording.mpCtx->GetConfig());
+                            vrc = mRecording.mAudioRec->applyConfiguration(mRecording.mCtx.GetConfig());
                             if (RT_SUCCESS(vrc))
                                 vrc = mRecording.mAudioRec->doAttachDriverViaEmt(ptrVM.rawUVM(), ptrVM.vtable(), pAutoLock);
@@ -5913 +5912 @@
 # endif
                         if (   RT_SUCCESS(vrc)
-                            && mRecording.mpCtx->IsReady()) /* Any video recording (audio and/or video) feature enabled? */
+                            && mRecording.mCtx.IsReady()) /* Any video recording (audio and/or video) feature enabled? */
                         {
                             vrc = pDisplay->i_recordingInvalidate();
@@ -7235 +7234 @@
 HRESULT Console::i_recordingSendAudio(const void *pvData, size_t cbData, uint64_t uTimestampMs)
 {
-    if (!mRecording.mpCtx)
-        return S_OK;
-
-    if (   mRecording.mpCtx->IsStarted()
-        && mRecording.mpCtx->IsFeatureEnabled(RecordingFeature_Audio))
-        return mRecording.mpCtx->SendAudioFrame(pvData, cbData, uTimestampMs);
+    if (   mRecording.mCtx.IsStarted()
+        && mRecording.mCtx.IsFeatureEnabled(RecordingFeature_Audio))
+        return mRecording.mCtx.SendAudioFrame(pvData, cbData, uTimestampMs);
 
     return S_OK;
@@ -7310 +7306 @@
 int Console::i_recordingCreate(void)
 {
-    AssertReturn(mRecording.mpCtx == NULL, VERR_WRONG_ORDER);
-
     settings::RecordingSettings recordingSettings;
-    int rc = i_recordingGetSettings(recordingSettings);
-    if (RT_SUCCESS(rc))
-    {
-        try
-        {
-            mRecording.mpCtx = new RecordingContext(this /* pConsole */, recordingSettings);
-        }
-        catch (std::bad_alloc &)
-        {
-            return VERR_NO_MEMORY;
-        }
-        catch (int &rc2)
-        {
-            return rc2;
-        }
-    }
-
-    LogFlowFuncLeaveRC(rc);
-    return rc;
+    int vrc = i_recordingGetSettings(recordingSettings);
+    if (RT_SUCCESS(vrc))
+        vrc = mRecording.mCtx.Create(this, recordingSettings);
+
+    LogFlowFuncLeaveRC(vrc);
+    return vrc;
 }
 
@@ -7339 +7320 @@
 void Console::i_recordingDestroy(void)
 {
-    if (mRecording.mpCtx)
-    {
-        delete mRecording.mpCtx;
-        mRecording.mpCtx = NULL;
-    }
-
-    LogFlowThisFuncLeave();
+    mRecording.mCtx.Destroy();
 }
 
@@ -7356 +7331 @@
 {
     RT_NOREF(pAutoLock);
-    AssertPtrReturn(mRecording.mpCtx, VERR_WRONG_ORDER);
-
-    if (mRecording.mpCtx->IsStarted())
+
+    if (mRecording.mCtx.IsStarted())
         return VINF_SUCCESS;
 
     LogRel(("Recording: Starting ...\n"));
 
-    int rc = mRecording.mpCtx->Start();
-    if (RT_SUCCESS(rc))
-    {
-        for (unsigned uScreen = 0; uScreen < mRecording.mpCtx->GetStreamCount(); uScreen++)
+    int vrc = mRecording.mCtx.Start();
+    if (RT_SUCCESS(vrc))
+    {
+        for (unsigned uScreen = 0; uScreen < mRecording.mCtx.GetStreamCount(); uScreen++)
             mDisplay->i_recordingScreenChanged(uScreen);
     }
 
-    LogFlowFuncLeaveRC(rc);
-    return rc;
+    LogFlowFuncLeaveRC(vrc);
+    return vrc;
 }
 
@@ -7379 +7353 @@
 int Console::i_recordingStop(util::AutoWriteLock *pAutoLock /* = NULL */)
 {
-    if (   !mRecording.mpCtx
-        || !mRecording.mpCtx->IsStarted())
+    if (!mRecording.mCtx.IsStarted())
         return VINF_SUCCESS;
 
     LogRel(("Recording: Stopping ...\n"));
 
-    int rc = mRecording.mpCtx->Stop();
-    if (RT_SUCCESS(rc))
-    {
-        const size_t cStreams = mRecording.mpCtx->GetStreamCount();
+    int vrc = mRecording.mCtx.Stop();
+    if (RT_SUCCESS(vrc))
+    {
+        const size_t cStreams = mRecording.mCtx.GetStreamCount();
         for (unsigned uScreen = 0; uScreen < cStreams; ++uScreen)
             mDisplay->i_recordingScreenChanged(uScreen);
@@ -7405 +7378 @@
     }
 
-    LogFlowFuncLeaveRC(rc);
-    return rc;
+    LogFlowFuncLeaveRC(vrc);
+    return vrc;
 }
 

trunk/src/VBox/Main/src-client/Recording.cpp

@@ -58 +58 @@
 
 
-RecordingContext::RecordingContext(Console *a_pConsole, const settings::RecordingSettings &a_Settings)
-    : pConsole(a_pConsole)
+/**
+ * Recording context constructor.
+ *
+ * @note    Will throw when unable to create.
+ */
+RecordingContext::RecordingContext(void)
+    : pConsole(NULL)
     , enmState(RECORDINGSTS_UNINITIALIZED)
     , cStreamsEnabled(0)
 {
-    int vrc = RecordingContext::createInternal(a_Settings);
+    int vrc = RTCritSectInit(&this->CritSect);
     if (RT_FAILURE(vrc))
         throw vrc;
 }
 
+/**
+ * Recording context constructor.
+ *
+ * @param   ptrConsole          Pointer to console object this context is bound to (weak pointer).
+ * @param   settings            Reference to recording settings to use for creation.
+ *
+ * @note    Will throw when unable to create.
+ */
+RecordingContext::RecordingContext(Console *ptrConsole, const settings::RecordingSettings &settings)
+    : pConsole(NULL)
+    , enmState(RECORDINGSTS_UNINITIALIZED)
+    , cStreamsEnabled(0)
+{
+    int vrc = RTCritSectInit(&this->CritSect);
+    if (RT_FAILURE(vrc))
+        throw vrc;
+
+    vrc = RecordingContext::createInternal(ptrConsole, settings);
+    if (RT_FAILURE(vrc))
+        throw vrc;
+}
+
 RecordingContext::~RecordingContext(void)
 {
     destroyInternal();
+
+    if (RTCritSectIsInitialized(&this->CritSect))
+        RTCritSectDelete(&this->CritSect);
 }
 
@@ -143 +173 @@
  *
  * @returns IPRT status code.
- * @param   a_Settings          Recording settings to use for context creation.
- */
-int RecordingContext::createInternal(const settings::RecordingSettings &a_Settings)
-{
-    int vrc = RTCritSectInit(&this->CritSect);
-    if (RT_FAILURE(vrc))
-        return vrc;
-
-    settings::RecordingScreenSettingsMap::const_iterator itScreen = a_Settings.mapScreens.begin();
-    while (itScreen != a_Settings.mapScreens.end())
+ * @param   ptrConsole          Pointer to console object this context is bound to (weak pointer).
+ * @param   settings            Reference to recording settings to use for creation.
+ */
+int RecordingContext::createInternal(Console *ptrConsole, const settings::RecordingSettings &settings)
+{
+    int vrc = VINF_SUCCESS;
+
+    this->pConsole = ptrConsole;
+
+    settings::RecordingScreenSettingsMap::const_iterator itScreen = settings.mapScreens.begin();
+    while (itScreen != settings.mapScreens.end())
     {
         RecordingStream *pStream = NULL;
@@ -178 +209 @@
 
         /* Copy the settings to our context. */
-        this->Settings  = a_Settings;
+        this->Settings  = settings;
 
         vrc = RTSemEventCreate(&this->WaitEvent);
@@ -260 +291 @@
 void RecordingContext::destroyInternal(void)
 {
+    lock();
+
     if (this->enmState == RECORDINGSTS_UNINITIALIZED)
+    {
+        unlock();
         return;
+    }
 
     int vrc = stopInternal();
     AssertRCReturnVoid(vrc);
-
-    lock();
 
     vrc = RTSemEventDestroy(this->WaitEvent);
@@ -292 +326 @@
     Assert(this->mapBlocksCommon.size() == 0);
 
-    unlock();
-
-    if (RTCritSectIsInitialized(&this->CritSect))
-        RTCritSectDelete(&this->CritSect);
-
     this->enmState = RECORDINGSTS_UNINITIALIZED;
+
+    unlock();
 }
 
@@ -371 +402 @@
  *
  * @returns IPRT status code.
- * @param   a_Settings          Recording settings to use for creation.
- *
- */
-int RecordingContext::Create(const settings::RecordingSettings &a_Settings)
-{
-    return createInternal(a_Settings);
+ * @param   ptrConsole          Pointer to console object this context is bound to (weak pointer).
+ * @param   settings            Reference to recording settings to use for creation.
+ */
+int RecordingContext::Create(Console *ptrConsole, const settings::RecordingSettings &settings)
+{
+    return createInternal(ptrConsole, settings);
 }