Index: /trunk/src/VBox/Frontends/VirtualBox/src/runtime/UIIndicatorsPool.cpp =================================================================== --- /trunk/src/VBox/Frontends/VirtualBox/src/runtime/UIIndicatorsPool.cpp (revision 75306) +++ /trunk/src/VBox/Frontends/VirtualBox/src/runtime/UIIndicatorsPool.cpp (revision 75307) @@ -957,17 +957,9 @@ /* For now all screens have the same config: */ CCaptureScreenSettings recordingScreen0Settings = recordingSettings.GetScreenSettings(0); - - QStringList strOptionsPairList = recordingScreen0Settings.GetOptions().split(",", QString::SkipEmptyParts); - - for (int i = 0; i < strOptionsPairList.size(); ++i) - { - if (strOptionsPairList.at(i).contains("vc_enabled", Qt::CaseInsensitive) && - strOptionsPairList.at(i).contains("true", Qt::CaseInsensitive)) - m_eRecordingMode = (UIIndicatorStateRecordingMode)((int)m_eRecordingMode | (int)UIIndicatorStateRecordingMode_Video); - - if (strOptionsPairList.at(i).contains("ac_enabled", Qt::CaseInsensitive) && - strOptionsPairList.at(i).contains("true", Qt::CaseInsensitive)) - m_eRecordingMode = (UIIndicatorStateRecordingMode)((int)m_eRecordingMode | (int)UIIndicatorStateRecordingMode_Audio); - } + if (recordingScreen0Settings.IsFeatureEnabled(KCaptureFeature_Video)) + m_eRecordingMode = (UIIndicatorStateRecordingMode)((int)m_eRecordingMode | (int)UIIndicatorStateRecordingMode_Video); + + if (recordingScreen0Settings.IsFeatureEnabled(KCaptureFeature_Audio)) + m_eRecordingMode = (UIIndicatorStateRecordingMode)((int)m_eRecordingMode | (int)UIIndicatorStateRecordingMode_Audio); } Index: /trunk/src/VBox/Frontends/VirtualBox/src/settings/machine/UIMachineSettingsDisplay.cpp =================================================================== --- /trunk/src/VBox/Frontends/VirtualBox/src/settings/machine/UIMachineSettingsDisplay.cpp (revision 75306) +++ /trunk/src/VBox/Frontends/VirtualBox/src/settings/machine/UIMachineSettingsDisplay.cpp (revision 75307) @@ -370,18 +370,19 @@ /* Check whether remote display server is valid: */ - const CVRDEServer &comServer = m_machine.GetVRDEServer(); - oldDisplayData.m_fRemoteDisplayServerSupported = !comServer.isNull(); - if (!comServer.isNull()) + const CVRDEServer &vrdeServer = m_machine.GetVRDEServer(); + oldDisplayData.m_fRemoteDisplayServerSupported = !vrdeServer.isNull(); + if (!vrdeServer.isNull()) { /* Gather old 'Remote Display' data: */ - oldDisplayData.m_fRemoteDisplayServerEnabled = comServer.GetEnabled(); - oldDisplayData.m_strRemoteDisplayPort = comServer.GetVRDEProperty("TCP/Ports"); - oldDisplayData.m_remoteDisplayAuthType = comServer.GetAuthType(); - oldDisplayData.m_uRemoteDisplayTimeout = comServer.GetAuthTimeout(); - oldDisplayData.m_fRemoteDisplayMultiConnAllowed = comServer.GetAllowMultiConnection(); + oldDisplayData.m_fRemoteDisplayServerEnabled = vrdeServer.GetEnabled(); + oldDisplayData.m_strRemoteDisplayPort = vrdeServer.GetVRDEProperty("TCP/Ports"); + oldDisplayData.m_remoteDisplayAuthType = vrdeServer.GetAuthType(); + oldDisplayData.m_uRemoteDisplayTimeout = vrdeServer.GetAuthTimeout(); + oldDisplayData.m_fRemoteDisplayMultiConnAllowed = vrdeServer.GetAllowMultiConnection(); } /* Gather old 'Recording' data: */ CCaptureSettings recordingSettings = m_machine.GetCaptureSettings(); + Assert(recordingSettings.isNotNull()); oldDisplayData.m_fRecordingEnabled = recordingSettings.GetEnabled(); @@ -403,6 +404,7 @@ for (int iScreenIndex = 0; iScreenIndex < recordingScreenSettingsVector.size(); ++iScreenIndex) { - CCaptureScreenSettings captureScreenSettings = recordingScreenSettingsVector.at(iScreenIndex); - oldDisplayData.m_vecRecordingScreens[iScreenIndex] = captureScreenSettings.GetEnabled(); + CCaptureScreenSettings recordingScreenSettings = recordingScreenSettingsVector.at(iScreenIndex); + if (!recordingScreenSettings.isNull()) + oldDisplayData.m_vecRecordingScreens[iScreenIndex] = recordingScreenSettings.GetEnabled(); } @@ -1510,4 +1512,5 @@ CCaptureSettings recordingSettings = m_machine.GetCaptureSettings(); + Assert(recordingSettings.isNotNull()); /* Save new 'Recording' data for online case: */ Index: /trunk/src/VBox/Main/include/CaptureScreenSettingsImpl.h =================================================================== --- /trunk/src/VBox/Main/include/CaptureScreenSettingsImpl.h (revision 75306) +++ /trunk/src/VBox/Main/include/CaptureScreenSettingsImpl.h (revision 75307) @@ -103,5 +103,6 @@ // internal methods bool i_canChangeSettings(); - int i_getDefaultCaptureFile(Utf8Str &strFile); + int i_getDefaultFileName(Utf8Str &strFile); + int i_initInternal(); private: Index: /trunk/src/VBox/Main/include/CaptureSettingsImpl.h =================================================================== --- /trunk/src/VBox/Main/include/CaptureSettingsImpl.h (revision 75306) +++ /trunk/src/VBox/Main/include/CaptureSettingsImpl.h (revision 75307) @@ -26,4 +26,5 @@ { struct CaptureSettings; + struct CaptureScreenSettings; } @@ -57,4 +58,8 @@ private: + int i_addScreen(uint32_t uScreenId, const settings::CaptureScreenSettings &data); + +private: + // wrapped ICaptureSettings properties HRESULT getEnabled(BOOL *enabled); Index: /trunk/src/VBox/Main/include/MachineImpl.h =================================================================== --- /trunk/src/VBox/Main/include/MachineImpl.h (revision 75306) +++ /trunk/src/VBox/Main/include/MachineImpl.h (revision 75307) @@ -494,4 +494,6 @@ void i_setModified(uint32_t fl, bool fAllowStateModification = true); void i_setModifiedLock(uint32_t fl, bool fAllowStateModification = true); + + MachineState_T i_getMachineState() const { return mData->mMachineState; } bool i_isStateModificationAllowed() const { return mData->m_fAllowStateModification; } Index: /trunk/src/VBox/Main/include/VideoRec.h =================================================================== --- /trunk/src/VBox/Main/include/VideoRec.h (revision 75306) +++ /trunk/src/VBox/Main/include/VideoRec.h (revision 75307) @@ -180,4 +180,7 @@ int Destroy(void); + int Start(void); + int Stop(void); + int SendAudioFrame(const void *pvData, size_t cbData, uint64_t uTimestampMs); int SendVideoFrame(uint32_t uScreen, @@ -196,4 +199,6 @@ int createInternal(const settings::CaptureSettings &a_Settings); + int startInternal(void); + int stopInternal(void); int destroyInternal(void); Index: /trunk/src/VBox/Main/include/VideoRecStream.h =================================================================== --- /trunk/src/VBox/Main/include/VideoRecStream.h (revision 75306) +++ /trunk/src/VBox/Main/include/VideoRecStream.h (revision 75307) @@ -130,5 +130,5 @@ protected: - int open(void); + int open(const settings::CaptureScreenSettings &Settings); int close(void); Index: /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 75306) +++ /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 75307) @@ -5616,8 +5616,8 @@ if (pDisplay) { - const bool fEnabled = Capture.mpVideoRecCtx - && Capture.mpVideoRecCtx->IsStarted(); - - if (RT_BOOL(fEnable) != fEnabled) + const bool fIsEnabled = Capture.mpVideoRecCtx + && Capture.mpVideoRecCtx->IsStarted(); + + if (RT_BOOL(fEnable) != fIsEnabled) { LogRel(("VideoRec: %s\n", fEnable ? "Enabling" : "Disabling")); @@ -5628,4 +5628,7 @@ { vrc = i_videoRecCreate(); + if (RT_SUCCESS(vrc)) + vrc = i_videoRecStart(); + if (RT_SUCCESS(vrc)) { @@ -6904,5 +6907,5 @@ AssertComRCReturn(hrc, VERR_INVALID_PARAMETER); - Settings.mapScreens.clear(); + Settings.applyDefaults(); for (unsigned long i = 0; i < (unsigned long)paCaptureScreens.size(); ++i) @@ -6934,4 +6937,6 @@ } + Assert(Settings.mapScreens.size() == paCaptureScreens.size()); + return VINF_SUCCESS; } @@ -6961,4 +6966,13 @@ } + settings::CaptureSettings Settings; + rc = i_videoRecGetSettings(Settings); + if (RT_SUCCESS(rc)) + { + AssertPtr(Capture.mpVideoRecCtx); + rc = Capture.mpVideoRecCtx->Create(Settings); + } + + LogFlowFuncLeaveRC(rc); return rc; } @@ -6970,5 +6984,10 @@ { if (Capture.mpVideoRecCtx) + { delete Capture.mpVideoRecCtx; + Capture.mpVideoRecCtx = NULL; + } + + LogFlowThisFuncLeave(); } @@ -6987,19 +7006,13 @@ LogRel(("VideoRec: Starting ...\n")); - settings::CaptureSettings Settings; - int rc = i_videoRecGetSettings(Settings); - if (RT_SUCCESS(rc)) - { - rc = Capture.mpVideoRecCtx->Create(Settings); - if (RT_SUCCESS(rc)) - { - for (unsigned uScreen = 0; uScreen < Capture.mpVideoRecCtx->GetStreamCount(); uScreen++) - mDisplay->i_videoRecScreenChanged(uScreen); - } - } + int rc = VINF_SUCCESS; + + for (unsigned uScreen = 0; uScreen < Capture.mpVideoRecCtx->GetStreamCount(); uScreen++) + mDisplay->i_videoRecScreenChanged(uScreen); if (RT_FAILURE(rc)) LogRel(("VideoRec: Failed to start video recording (%Rrc)\n", rc)); + LogFlowFuncLeaveRC(rc); return rc; } @@ -7010,7 +7023,6 @@ int Console::i_videoRecStop(void) { - AssertPtrReturn(Capture.mpVideoRecCtx, VERR_WRONG_ORDER); - - if (!Capture.mpVideoRecCtx->IsStarted()) + if ( !Capture.mpVideoRecCtx + || !Capture.mpVideoRecCtx->IsStarted()) return VINF_SUCCESS; @@ -7020,7 +7032,4 @@ for (unsigned uScreen = 0; uScreen < cStreams; ++uScreen) mDisplay->i_videoRecScreenChanged(uScreen); - - delete Capture.mpVideoRecCtx; - Capture.mpVideoRecCtx = NULL; ComPtr pCaptureSettings; @@ -7032,4 +7041,5 @@ LogRel(("VideoRec: Stopped\n")); + LogFlowFuncLeaveRC(VINF_SUCCESS); return VINF_SUCCESS; } Index: /trunk/src/VBox/Main/src-client/VideoRec.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/VideoRec.cpp (revision 75306) +++ /trunk/src/VBox/Main/src-client/VideoRec.cpp (revision 75307) @@ -58,5 +58,5 @@ /** - * Enumeration for a video recording state. + * Enumeration for a recording state. */ enum VIDEORECSTS @@ -64,6 +64,8 @@ /** Not initialized. */ VIDEORECSTS_UNINITIALIZED = 0, - /** Initialized. */ - VIDEORECSTS_INITIALIZED = 1, + /** Created. */ + VIDEORECSTS_CREATED = 1, + /** Started. */ + VIDEORECSTS_STARTED = 2, /** The usual 32-bit hack. */ VIDEORECSTS_32BIT_HACK = 0x7fffffff @@ -105,8 +107,10 @@ CaptureContext::CaptureContext(Console *a_pConsole) - : pConsole(a_pConsole) { } + : pConsole(a_pConsole) + , enmState(VIDEORECSTS_UNINITIALIZED) { } CaptureContext::CaptureContext(Console *a_pConsole, const settings::CaptureSettings &a_Settings) : pConsole(a_pConsole) + , enmState(VIDEORECSTS_UNINITIALIZED) { int rc = CaptureContext::createInternal(a_Settings); @@ -214,5 +218,5 @@ { this->tsStartMs = RTTimeMilliTS(); - this->enmState = VIDEORECSTS_UNINITIALIZED; + this->enmState = VIDEORECSTS_CREATED; this->fStarted = false; this->fShutdown = false; @@ -223,16 +227,4 @@ rc = RTSemEventCreate(&this->WaitEvent); AssertRCReturn(rc, rc); - - rc = RTThreadCreate(&this->Thread, CaptureContext::threadMain, (void *)this, 0, - RTTHREADTYPE_MAIN_WORKER, RTTHREADFLAGS_WAITABLE, "VideoRec"); - - if (RT_SUCCESS(rc)) /* Wait for the thread to start. */ - rc = RTThreadUserWait(this->Thread, 30 * RT_MS_1SEC /* 30s timeout */); - - if (RT_SUCCESS(rc)) - { - this->enmState = VIDEORECSTS_INITIALIZED; - this->fStarted = true; - } } @@ -246,4 +238,44 @@ } +int CaptureContext::startInternal(void) +{ + if (this->enmState == VIDEORECSTS_STARTED) + return VINF_SUCCESS; + + Assert(this->enmState == VIDEORECSTS_CREATED); + + int rc = RTThreadCreate(&this->Thread, CaptureContext::threadMain, (void *)this, 0, + RTTHREADTYPE_MAIN_WORKER, RTTHREADFLAGS_WAITABLE, "VideoRec"); + + if (RT_SUCCESS(rc)) /* Wait for the thread to start. */ + rc = RTThreadUserWait(this->Thread, 30 * RT_MS_1SEC /* 30s timeout */); + + if (RT_SUCCESS(rc)) + { + this->enmState = VIDEORECSTS_STARTED; + this->fStarted = true; + } + + return rc; +} + +int CaptureContext::stopInternal(void) +{ + if (this->enmState != VIDEORECSTS_STARTED) + return VINF_SUCCESS; + + LogFunc(("Shutting down thread ...\n")); + + /* Set shutdown indicator. */ + ASMAtomicWriteBool(&this->fShutdown, true); + + /* Signal the thread and wait for it to shut down. */ + int rc = threadNotify(); + if (RT_SUCCESS(rc)) + rc = RTThreadWait(this->Thread, 30 * 1000 /* 10s timeout */, NULL); + + return rc; +} + /** * Destroys a video recording context. @@ -253,16 +285,7 @@ int rc = VINF_SUCCESS; - if (this->enmState == VIDEORECSTS_INITIALIZED) - { - LogFunc(("Shutting down thread ...\n")); - - /* Set shutdown indicator. */ - ASMAtomicWriteBool(&this->fShutdown, true); - - /* Signal the thread and wait for it to shut down. */ - rc = threadNotify(); - if (RT_SUCCESS(rc)) - rc = RTThreadWait(this->Thread, 30 * 1000 /* 10s timeout */, NULL); - + if (this->enmState == VIDEORECSTS_STARTED) + { + rc = stopInternal(); if (RT_SUCCESS(rc)) { @@ -365,4 +388,14 @@ } +int CaptureContext::Start(void) +{ + return startInternal(); +} + +int CaptureContext::Stop(void) +{ + return stopInternal(); +} + bool CaptureContext::IsFeatureEnabled(CaptureFeature_T enmFeature) const { @@ -394,5 +427,5 @@ RT_NOREF(uTimeStampMs); - if (this->enmState != VIDEORECSTS_INITIALIZED) + if (this->enmState != VIDEORECSTS_STARTED) return false; Index: /trunk/src/VBox/Main/src-client/VideoRecStream.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/VideoRecStream.cpp (revision 75306) +++ /trunk/src/VBox/Main/src-client/VideoRecStream.cpp (revision 75307) @@ -45,4 +45,6 @@ : tsStartMs(0) { + File.pWEBM = NULL; + File.hFile = NIL_RTFILE; } @@ -50,4 +52,7 @@ : tsStartMs(0) { + File.pWEBM = NULL; + File.hFile = NIL_RTFILE; + int rc2 = initInternal(uScreen, Settings); if (RT_FAILURE(rc2)) @@ -66,17 +71,18 @@ * @returns IPRT status code. */ -int CaptureStream::open(void) -{ - Assert(ScreenSettings.enmDest == CaptureDestination_None); +int CaptureStream::open(const settings::CaptureScreenSettings &Settings) +{ + /* Sanity. */ + Assert(Settings.enmDest != CaptureDestination_None); int rc; - switch (ScreenSettings.enmDest) + switch (Settings.enmDest) { case CaptureDestination_File: { - Assert(ScreenSettings.File.strName.isNotEmpty()); - - char *pszAbsPath = RTPathAbsDup(ScreenSettings.File.strName.c_str()); + Assert(Settings.File.strName.isNotEmpty()); + + char *pszAbsPath = RTPathAbsDup(Settings.File.strName.c_str()); AssertPtrReturn(pszAbsPath, VERR_NO_MEMORY); @@ -132,4 +138,14 @@ if (RT_SUCCESS(rc)) rc = RTFileOpen(&hFile, pszFile, fOpen); + } + + try + { + Assert(File.pWEBM == NULL); + File.pWEBM = new WebMWriter(); + } + catch (std::bad_alloc &) + { + rc = VERR_NO_MEMORY; } @@ -656,5 +672,5 @@ return rc; - rc = open(); + rc = open(Settings); if (RT_FAILURE(rc)) return rc; @@ -675,4 +691,5 @@ const char *pszFile = this->ScreenSettings.File.strName.c_str(); + AssertPtr(File.pWEBM); rc = File.pWEBM->OpenEx(pszFile, &this->File.hFile, #ifdef VBOX_WITH_AUDIO_VIDEOREC Index: /trunk/src/VBox/Main/src-server/CaptureScreenSettingsImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-server/CaptureScreenSettingsImpl.cpp (revision 75306) +++ /trunk/src/VBox/Main/src-server/CaptureScreenSettingsImpl.cpp (revision 75307) @@ -86,6 +86,7 @@ m = new Data(); - /* share the parent weakly */ + /* Share the parent weakly. */ unconst(m->pMachine) = aParent; + /* mPeer is left null. */ /* Simply copy the settings data. */ @@ -94,8 +95,19 @@ m->bd->operator=(data); - autoInitSpan.setSucceeded(); + HRESULT rc = S_OK; + + int vrc = i_initInternal(); + if (RT_SUCCESS(vrc)) + { + autoInitSpan.setSucceeded(); + } + else + { + autoInitSpan.setFailed(); + rc = E_UNEXPECTED; + } LogFlowThisFuncLeave(); - return S_OK; + return rc; } @@ -129,8 +141,19 @@ m->bd.share(that->m->bd); - autoInitSpan.setSucceeded(); + HRESULT rc = S_OK; + + int vrc = i_initInternal(); + if (RT_SUCCESS(vrc)) + { + autoInitSpan.setSucceeded(); + } + else + { + autoInitSpan.setFailed(); + rc = E_UNEXPECTED; + } LogFlowThisFuncLeave(); - return S_OK; + return rc; } @@ -154,5 +177,5 @@ unconst(m->pMachine) = aParent; - // mPeer is left null + /* mPeer is left null. */ AutoWriteLock thatlock(that COMMA_LOCKVAL_SRC_POS); @@ -161,8 +184,19 @@ m->bd.attachCopy(that->m->bd); - autoInitSpan.setSucceeded(); + HRESULT rc = S_OK; + + int vrc = i_initInternal(); + if (RT_SUCCESS(vrc)) + { + autoInitSpan.setSucceeded(); + } + else + { + autoInitSpan.setFailed(); + rc = E_UNEXPECTED; + } LogFlowThisFuncLeave(); - return S_OK; + return rc; } @@ -216,14 +250,14 @@ if (m->bd->fEnabled != RT_BOOL(enabled)) { + m->bd.backup(); + m->bd->fEnabled = RT_BOOL(enabled); alock.release(); + + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); HRESULT rc = m->pMachine->i_onCaptureChange(); if (FAILED(rc)) return rc; - - m->pMachine->i_setModified(Machine::IsModified_Capture); - - alock.acquire(); - m->bd->fEnabled = RT_BOOL(enabled); - alock.release(); /** Save settings if online - @todo why is this required? -- @bugref{6818} */ @@ -263,7 +297,5 @@ AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->featureMap.clear(); @@ -273,4 +305,10 @@ m->bd->featureMap[CaptureFeature_Video] = true; + alock.release(); + + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -292,8 +330,13 @@ AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - m->pMachine->i_setModified(Machine::IsModified_Capture); m->bd.backup(); m->bd->enmDest = aDestination; + alock.release(); + + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -303,8 +346,5 @@ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - if (m->bd->File.strName.isEmpty()) - i_getDefaultCaptureFile(aFileName); - else - aFileName = m->bd->File.strName; + aFileName = m->bd->File.strName; return S_OK; @@ -323,16 +363,13 @@ return setError(E_INVALIDARG, tr("Capture file name '%s' is not absolute"), strFile.c_str()); - if (!strFile.isEmpty()) - { - Utf8Str defaultFile; - i_getDefaultCaptureFile(defaultFile); - if (!RTPathCompare(strFile.c_str(), defaultFile.c_str())) - strFile.setNull(); - } - - m->pMachine->i_setModified(Machine::IsModified_Capture); m->bd.backup(); m->bd->File.strName = strFile; + alock.release(); + + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -354,9 +391,11 @@ return setError(E_INVALIDARG, tr("Cannot change maximum time while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->ulMaxTimeS = aMaxTimeS; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -378,9 +417,11 @@ return setError(E_INVALIDARG, tr("Cannot change maximum file size while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->File.ulMaxSizeMB = aMaxFileSize; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -402,9 +443,11 @@ return setError(E_INVALIDARG, tr("Cannot change options string while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->strOptions = aOptions; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -426,9 +469,11 @@ return setError(E_INVALIDARG, tr("Cannot change audio codec while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Audio.enmAudioCodec = aCodec; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -450,9 +495,11 @@ return setError(E_INVALIDARG, tr("Cannot change audio Hertz rate while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Audio.uHz = (uint16_t)aHz; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -474,9 +521,11 @@ return setError(E_INVALIDARG, tr("Cannot change audio bits while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Audio.cBits = (uint8_t)aBits; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -498,9 +547,11 @@ return setError(E_INVALIDARG, tr("Cannot change audio channels while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Audio.cChannels = (uint8_t)aChannels; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -522,9 +573,11 @@ return setError(E_INVALIDARG, tr("Cannot change video codec while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Video.enmCodec = aCodec; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -546,9 +599,11 @@ return setError(E_INVALIDARG, tr("Cannot change video width while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Video.ulWidth = aVideoWidth; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -570,9 +625,11 @@ return setError(E_INVALIDARG, tr("Cannot change video height while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Video.ulHeight = aVideoHeight; + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); + return S_OK; } @@ -594,8 +651,10 @@ return setError(E_INVALIDARG, tr("Cannot change video rate while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Video.ulRate = aVideoRate; + + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); return S_OK; @@ -640,8 +699,10 @@ return setError(E_INVALIDARG, tr("Cannot change video FPS while capturing is enabled")); - m->pMachine->i_setModified(Machine::IsModified_Capture); - m->bd.backup(); - + m->bd.backup(); m->bd->Video.ulFPS = aVideoFPS; + + AutoWriteLock mlock(m->pMachine COMMA_LOCKVAL_SRC_POS); + m->pMachine->i_setModified(Machine::IsModified_Capture); + mlock.release(); return S_OK; @@ -685,5 +746,5 @@ * Returns the full path to the default video capture file. */ -int CaptureScreenSettings::i_getDefaultCaptureFile(Utf8Str &strFile) +int CaptureScreenSettings::i_getDefaultFileName(Utf8Str &strFile) { AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); @@ -696,2 +757,13 @@ } +/** + * Initializes data, internal version. + * + * @returns IPRT status code. + */ +int CaptureScreenSettings::i_initInternal(void) +{ + Assert(m); + return i_getDefaultFileName(m->bd->File.strName); +} + Index: /trunk/src/VBox/Main/src-server/CaptureSettingsImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-server/CaptureSettingsImpl.cpp (revision 75306) +++ /trunk/src/VBox/Main/src-server/CaptureSettingsImpl.cpp (revision 75307) @@ -44,4 +44,5 @@ ComObjPtr pPeer; CaptureScreenSettingsMap mapScreenSettings; + bool fHasMachineLock; // use the XML settings structure in the members for simplicity @@ -84,4 +85,5 @@ m->bd.allocate(); + m->fHasMachineLock = false; autoInitSpan.setSucceeded(); @@ -119,4 +121,5 @@ m->bd.share(that->m->bd); m->mapScreenSettings = that->m->mapScreenSettings; + m->fHasMachineLock = false; autoInitSpan.setSucceeded(); @@ -151,4 +154,5 @@ m->bd.attachCopy(that->m->bd); m->mapScreenSettings = that->m->mapScreenSettings; + m->fHasMachineLock = false; autoInitSpan.setSucceeded(); @@ -196,8 +200,4 @@ HRESULT CaptureSettings::setEnabled(BOOL enable) { - /* the machine needs to be mutable */ - AutoMutableStateDependency adep(m->pMachine); - if (FAILED(adep.rc())) return adep.rc(); - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); @@ -216,6 +216,6 @@ { /* - * Normally we would do the actual change _after_ i_onVideoCaptureChange() succeeded. - * We cannot do this because that function uses Machine::GetVideoCaptureEnabled to + * Normally we would do the actual change _after_ i_onCaptureChange() succeeded. + * We cannot do this because that function uses CaptureSettings::GetEnabled to * determine if it should start or stop capturing. Therefore we need to manually * undo change. @@ -230,7 +230,13 @@ m->pMachine->i_setModified(Machine::IsModified_Capture); + /* We need to indicate here that we just took the machine lock, as Machine::i_saveSettings() will + * call i_commit(), which in turn also wants to lock the machine for writing. */ + m->fHasMachineLock = true; + /** Save settings if online - @todo why is this required? -- @bugref{6818} */ - if (Global::IsOnline(adep.machineState())) - m->pMachine->i_saveSettings(NULL); + if (Global::IsOnline(m->pMachine->i_getMachineState())) + rc = m->pMachine->i_saveSettings(NULL); + + m->fHasMachineLock = false; } } @@ -251,8 +257,11 @@ { itScreenSettings->second.queryInterfaceTo(aCaptureScreenSettings[i].asOutParam()); + Assert(aCaptureScreenSettings[i].isNotNull()); ++i; ++itScreenSettings; } + Assert(aCaptureScreenSettings.size() == m->mapScreenSettings.size()); + return S_OK; } @@ -280,4 +289,42 @@ /** + * Adds a screen settings object to the recording settings. + * + * @returns IPRT status code. VERR_ALREADY_EXISTS if the object in question already exists. + * @param uScreenId Screen ID to add settings for. + * @param data Recording screen settings to use for that screen. + */ +int CaptureSettings::i_addScreen(uint32_t uScreenId, const settings::CaptureScreenSettings &data) +{ + if (m->mapScreenSettings.find(uScreenId) != m->mapScreenSettings.end()) + { + AssertFailed(); + return VERR_ALREADY_EXISTS; + } + + int vrc = VINF_SUCCESS; + + ComObjPtr captureScreenSettings; + HRESULT rc = captureScreenSettings.createObject(); + if (SUCCEEDED(rc)) + { + rc = captureScreenSettings->init(m->pMachine, uScreenId, data); + if (SUCCEEDED(rc)) + { + try + { + m->mapScreenSettings[uScreenId] = captureScreenSettings; + } + catch (std::bad_alloc &) + { + vrc = VERR_NO_MEMORY; + } + } + } + + return vrc; +} + +/** * Loads settings from the given settings. * May be called once right after this object creation. @@ -301,17 +348,16 @@ while (itScreen != data.mapScreens.end()) { - ComObjPtr captureScreenSettings; - rc = captureScreenSettings.createObject(); - if (FAILED(rc)) + int vrc = i_addScreen(itScreen->first /* uScreenId */, itScreen->second /* Settings */); + if (RT_FAILURE(vrc)) + { + rc = E_OUTOFMEMORY; break; - - rc = captureScreenSettings->init(m->pMachine, itScreen->first /* uScreenId */, itScreen->second /* Settings */); - if (FAILED(rc)) - break; - - m->mapScreenSettings[itScreen->first] = captureScreenSettings; + } ++itScreen; } + + if (FAILED(rc)) + return rc; ComAssertComRC(rc); @@ -378,5 +424,7 @@ { /* attach new data to the peer and reshare it */ - AutoWriteLock peerlock(m->pPeer COMMA_LOCKVAL_SRC_POS); + AutoWriteLock peerlock(RT_SRC_POS); + if (!m->fHasMachineLock) + peerlock.attach(m->pPeer); m->pPeer->m->bd.attach(m->bd); } Index: /trunk/src/VBox/Main/xml/Settings.cpp =================================================================== --- /trunk/src/VBox/Main/xml/Settings.cpp (revision 75306) +++ /trunk/src/VBox/Main/xml/Settings.cpp (revision 75307) @@ -2387,9 +2387,10 @@ */ - fEnabled = true; + fEnabled = false; enmDest = CaptureDestination_File; ulMaxTimeS = 0; strOptions = ""; File.ulMaxSizeMB = 0; + File.strName = ""; Video.enmCodec = CaptureVideoCodec_VP8; Video.ulWidth = 1024; @@ -2404,6 +2405,4 @@ featureMap[CaptureFeature_Video] = true; featureMap[CaptureFeature_Audio] = false; - - //i_calculateFullPath } @@ -2413,20 +2412,19 @@ bool CaptureScreenSettings::areDefaultSettings(void) const { - const bool fDefault = fEnabled == true - && enmDest == CaptureDestination_File - && ulMaxTimeS == 0 - && strOptions == "" - && File.ulMaxSizeMB == 0 - && Video.enmCodec == CaptureVideoCodec_VP8 - && Video.ulWidth == 1024 - && Video.ulHeight == 768 - && Video.ulRate == 512 - && Video.ulFPS == 25 - && Audio.enmAudioCodec == CaptureAudioCodec_Opus - && Audio.cBits == 16 - && Audio.cChannels == 2 - && Audio.uHz == 22050; - - return fDefault; + return fEnabled == false + && enmDest == CaptureDestination_File + && ulMaxTimeS == 0 + && strOptions == "" + && File.ulMaxSizeMB == 0 + && File.strName == "" + && Video.enmCodec == CaptureVideoCodec_VP8 + && Video.ulWidth == 1024 + && Video.ulHeight == 768 + && Video.ulRate == 512 + && Video.ulFPS == 25 + && Audio.enmAudioCodec == CaptureAudioCodec_Opus + && Audio.cBits == 16 + && Audio.cChannels == 2 + && Audio.uHz == 22050; } @@ -2472,4 +2470,7 @@ } +/** + * Applies the default settings. + */ void CaptureSettings::applyDefaults(void) { @@ -2478,8 +2479,13 @@ mapScreens.clear(); - CaptureScreenSettings screenSettings; /* Apply default settings. */ - for (uint32_t l = 0; l < SchemaDefs::MaxGuestMonitors; l++) - { - mapScreens[l] = screenSettings; + try + { + /* Always add screen 0 to the default configuration. */ + CaptureScreenSettings screenSettings; /* Apply default settings for screen 0. */ + mapScreens[0] = screenSettings; + } + catch (std::bad_alloc &) + { + AssertFailed(); } } @@ -2490,14 +2496,12 @@ bool CaptureSettings::areDefaultSettings() const { + const bool fDefault = fEnabled == false + && mapScreens.size() == 1; + if (!fDefault) + return false; + CaptureScreenMap::const_iterator itScreen = mapScreens.begin(); - while (itScreen != mapScreens.end()) - { - if (!itScreen->second.areDefaultSettings()) - return false; - - ++itScreen; - } - - return true; + return itScreen->first == 0 + && itScreen->second.areDefaultSettings(); } @@ -4413,4 +4417,5 @@ /* At the moment we only support one capturing configuration, that is, all screens * have the same configuration. So load/save to/from screen 0. */ + Assert(hw.captureSettings.mapScreens.size()); /* At least screen must be present. */ CaptureScreenSettings &screen0Settings = hw.captureSettings.mapScreens[0]; @@ -4424,5 +4429,5 @@ pelmHwChild->getAttributeValue("fps", screen0Settings.Video.ulFPS); - for (unsigned i = 0; i < 64; i++) + for (unsigned i = 0; i < hw.cMonitors; i++) /* Don't add more settings than we have monitors configured. */ { if (u64VideoCaptureScreens & RT_BIT_64(i)) /* Screen i enabled? */ @@ -5839,4 +5844,5 @@ Assert(hw.captureSettings.mapScreens.size()); const CaptureScreenMap::const_iterator itScreen0Settings = hw.captureSettings.mapScreens.find(0); + Assert(itScreen0Settings != hw.captureSettings.mapScreens.end()); if (itScreen0Settings->second.ulMaxTimeS)