Release camera/video device on source removal, not on destruction

Move the capture-device / media-player teardown into VideoImpl::freeResources()
(now overridden by CameraImpl and VideoPlayerImpl) so it can run without
destroying the impl object, and route the destructors and loadMovie() through
it (re-entrant, null-safe).

Add reversible Source::releaseResources()/reacquireResources() hooks. Video
releases its device via unloadMovie() and re-opens it via build() (which reuses
the stored URI/device id). MappingManager::removeSource() now releases a removed
source's device immediately — even though the source may linger in the undo
stack — and RemoveSourceCommand::undo() re-acquires it. This lets the same
camera be re-opened within a session, replacing the old explicit-destructor
hack that caused the double-free crash.

Also call removeSource() unconditionally instead of inside Q_ASSERT(), which is
compiled out in release builds and would otherwise skip the removal entirely.

Author: aalex

src/core/CameraImpl.cpp

@@ -31,18 +31,30 @@ CameraImpl::CameraImpl()
 }
 
 CameraImpl::~CameraImpl()
+{
+  freeResources();
+}
+
+void CameraImpl::freeResources()
 {
   if (_camera)
     _camera->stop();
   delete _camera;
+  _camera = nullptr;
   delete _captureSession;
+  _captureSession = nullptr;
   delete _cameraSurface;
+  _cameraSurface = nullptr;
+  VideoImpl::freeResources();
 }
 
 bool CameraImpl::loadMovie(const QString &deviceId)
 {
   VideoImpl::loadMovie(deviceId);
 
+  // Release any previously-opened device before re-acquiring.
+  freeResources();
+
   // Find the camera device matching the given ID.
   QCameraDevice dev;
   for (const QCameraDevice& d : QMediaDevices::videoInputs()) {

src/core/CameraImpl.h

@@ -48,6 +48,10 @@ class CameraImpl : public VideoImpl
   bool hasBits() const override { return _cameraSurface && _cameraSurface->isActive(); }
   bool bitsHaveChanged() const override { return true; }
 
+protected:
+  // Stops and releases the capture device (called by unloadMovie() and dtor).
+  void freeResources() override;
+
 private:
   QCamera                *_camera;
   QMediaCaptureSession   *_captureSession;

src/core/Commands.cpp

@@ -270,6 +270,10 @@ void RemoveSourceCommand::undo()
   if (!_source.isNull())
   {
     MappingManager& manager = _mainWindow->getMappingManager();
+
+    // Re-open any device/player released when the source was removed.
+    _source->reacquireResources();
+
     uid lastId = manager.addSource(_source);
     _mainWindow->addSourceItem(lastId, _source->getIcon(), _source->getName());
 

src/core/MappingManager.cpp

@@ -133,6 +133,11 @@ bool MappingManager::removeSource(uid sourceId)
     Q_ASSERT(idx != -1);
     sourceVector.remove(idx);
     sourceMap.remove(sourceId);
+
+    // Release the source's heavy external resources (e.g. a camera device)
+    // immediately, even though the object itself may live on in the undo stack.
+    // reacquireResources() restores them if the removal is undone.
+    source->releaseResources();
     return true;
   }
   else

src/core/Source.cpp

@@ -268,6 +268,22 @@ void Video::rewind()
   _impl->resetMovie();
 }
 
+void Video::releaseResources()
+{
+  // Release the capture device / media player but keep the impl object, so the
+  // device is freed as soon as the source is removed (e.g. to let the same
+  // camera be re-opened) without invalidating the source. See reacquireResources().
+  if (_impl)
+    _impl->unloadMovie();
+}
+
+void Video::reacquireResources()
+{
+  // Re-open using the stored URI/device id (set by loadMovie()).
+  if (_impl)
+    _impl->build();
+}
+
 void Video::lockMutex() {
   _impl->lockMutex();
 }

src/core/Source.h

@@ -101,6 +101,14 @@ class Source : public Element
   /// Rewinds.
   virtual void rewind() {}
 
+  /// Releases heavy external resources (capture devices, players, network
+  /// clients) while keeping the object valid — e.g. when the source is removed
+  /// but kept in the undo history. Reversible via reacquireResources().
+  virtual void releaseResources() {}
+
+  /// Re-acquires resources released by releaseResources() (e.g. on undo).
+  virtual void reacquireResources() {}
+
   /// Locks mutex (default = no effect).
   virtual void lockMutex() {}
 
@@ -338,6 +346,11 @@ class Video : public Texture
   /// Rewinds.
   virtual void rewind();
 
+  /// Releases the capture device / player (keeps the object valid).
+  virtual void releaseResources();
+  /// Re-opens the capture device / player released by releaseResources().
+  virtual void reacquireResources();
+
   /// Locks mutex (default = no effect).
   virtual void lockMutex();
 

src/core/VideoImpl.h

@@ -99,6 +99,10 @@ class VideoImpl
 
   void resetMovie();
 
+  /// Unloads the current movie and releases the capture device / player
+  /// (reversible via build(), which re-opens the stored URI/device).
+  void unloadMovie();
+
   /// Locks mutex.
   void lockMutex();
   /// Unlocks mutex.
@@ -108,7 +112,6 @@ class VideoImpl
   bool waitForNextBits(int timeout, const uchar** bits = nullptr);
 
 protected:
-  void unloadMovie();
   virtual void freeResources();
 
   void _setMovieReady(bool ready) { _movieReady = ready; }

src/core/VideoPlayerImpl.cpp

@@ -24,29 +24,31 @@ VideoPlayerImpl::VideoPlayerImpl()
 }
 
 VideoPlayerImpl::~VideoPlayerImpl()
+{
+  freeResources();
+}
+
+void VideoPlayerImpl::freeResources()
 {
   if (_player) {
     _player->stop();
     delete _player;
+    _player = nullptr;
   }
   delete _audioOutput;
+  _audioOutput = nullptr;
   delete _videoSink;
+  _videoSink = nullptr;
+  _eos = false;
+  VideoImpl::freeResources();
 }
 
 bool VideoPlayerImpl::loadMovie(const QString& path)
 {
   // Store URI in base class.
   VideoImpl::loadMovie(path);
 
-  // Tear down any previous player.
-  if (_player) {
-    _player->stop();
-    delete _player;
-    delete _audioOutput;
-    delete _videoSink;
-  }
-
-  _eos = false;
+  // Tear down any previous player and reset frame state.
   freeResources();
 
   _player      = new QMediaPlayer(this);

src/core/VideoPlayerImpl.h

@@ -47,6 +47,10 @@ class VideoPlayerImpl : public QObject, public VideoImpl
 
   void update() override;
 
+protected:
+  // Releases the player/audio/sink (called by unloadMovie() and the destructor).
+  void freeResources() override;
+
 private slots:
   void onVideoFrameChanged(const QVideoFrame& frame);
   void onMediaStatusChanged(QMediaPlayer::MediaStatus status);

src/gui/MainWindow.cpp

@@ -3380,8 +3380,12 @@ void MainWindow::removeSourceItem(uid sourceId)
        it != sourceLayers.constEnd(); ++it) {
     removeLayerItem(it.key());
   }
-  // Remove source from model.
-  Q_ASSERT( mappingManager->removeSource(sourceId) );
+  // Remove source from model. Call unconditionally (not inside Q_ASSERT, which
+  // is compiled out in release builds and would skip the removal + the source's
+  // resource release).
+  bool sourceRemoved = mappingManager->removeSource(sourceId);
+  Q_ASSERT(sourceRemoved);
+  Q_UNUSED(sourceRemoved);
 
   // Remove associated mapper.
   sourcePropertyPanel->removeWidget(sourceGuis[sourceId]->getPropertiesEditor());