Merge pull request #43 from gift-surg/14-unable-to-free-blackmagic-vi…

…deo-source

Improvements to Blackmagic video source

.gitlab-ci.yml

@@ -443,6 +443,7 @@ pypi-blackmagic-decklink-sdi-4k:
     - this-branch-should-never-exist
     - 23-support-for-blackmagic-decklink-4k-extreme-12g
     - 20-support-for-capturing-bgra-frames-with-blackmagic-devices
+    - 14-unable-to-free-blackmagic-video-source
 
 blackmagic-decklink-sdi-4k:
   stage: test_cmake
@@ -464,6 +465,7 @@ blackmagic-decklink-sdi-4k:
     - this-branch-should-never-exist
     - 23-support-for-blackmagic-decklink-4k-extreme-12g
     - 20-support-for-capturing-bgra-frames-with-blackmagic-devices
+    - 14-unable-to-free-blackmagic-video-source
 
 ################## Device: Blackmagic DeckLink 4K Extreme 12G ##################
 pypi-blackmagic-decklink-4k-extreme-12g:
@@ -507,6 +509,7 @@ pypi-blackmagic-decklink-4k-extreme-12g:
     - 23-support-for-blackmagic-decklink-4k-extreme-12g
     - 20-support-for-capturing-bgra-frames-with-blackmagic-devices
     - 32-videosourcefactory-destructor-does-not-free-all-devices
+    - 14-unable-to-free-blackmagic-video-source
 
 blackmagic-decklink-4k-extreme-12g:
   stage: test_cmake
@@ -527,3 +530,4 @@ blackmagic-decklink-4k-extreme-12g:
     - 23-support-for-blackmagic-decklink-4k-extreme-12g
     - 20-support-for-capturing-bgra-frames-with-blackmagic-devices
     - 32-videosourcefactory-destructor-does-not-free-all-devices
+    - 14-unable-to-free-blackmagic-video-source

src/blackmagicsdk/blackmagicsdk_video_source.cpp

@@ -118,11 +118,14 @@ VideoSourceBlackmagicSDK::VideoSourceBlackmagicSDK(size_t deck_link_index,
 
 VideoSourceBlackmagicSDK::~VideoSourceBlackmagicSDK()
 {
-    // Make sure streamer thread not trying to access buffer
-    std::lock_guard<std::mutex> data_lock_guard(_data_lock);
-    _running = false;
+    { // Artificial scope for data lock
+        // Make sure streamer thread not trying to access buffer
+        std::lock_guard<std::mutex> data_lock_guard(_data_lock);
+        _running = false;
+    }
 
     // Stop streaming and disable enabled inputs
+    _deck_link_input->SetCallback(nullptr);
     _deck_link_input->StopStreams();
     _deck_link_input->DisableVideoInput();
 
@@ -217,6 +220,10 @@ HRESULT STDMETHODCALLTYPE VideoSourceBlackmagicSDK::VideoInputFrameArrived(
     IDeckLinkAudioInputPacket * audio_packet
 )
 {
+    if (not _running)
+        // nop if not running!
+        return S_OK;
+
     // Not processing the audio packet, but only the video
     // frame for the time being
     if (video_frame == nullptr)
@@ -226,34 +233,37 @@ HRESULT STDMETHODCALLTYPE VideoSourceBlackmagicSDK::VideoInputFrameArrived(
     // Nr. of bytes of received data
     size_t n_bytes = video_frame->GetRowBytes() * video_frame->GetHeight();
 
-    // Make sure only this thread is accessing the buffer now
-    std::lock_guard<std::mutex> data_lock_guard(_data_lock);
-
-    // Extend buffer if more memory needed than already allocated
-    if (n_bytes > _video_buffer_length)
-        _video_buffer = reinterpret_cast<uint8_t *>(
-                    realloc(_video_buffer, n_bytes * sizeof(uint8_t))
+    { // Artificial scope for data lock
+        // Make sure only this thread is accessing the buffer now
+        std::lock_guard<std::mutex> data_lock_guard(_data_lock);
+
+        // Extend buffer if more memory needed than already allocated
+        if (n_bytes > _video_buffer_length)
+            _video_buffer = reinterpret_cast<uint8_t *>(
+                        realloc(_video_buffer, n_bytes * sizeof(uint8_t))
+            );
+        if (_video_buffer == nullptr) // something's terribly wrong!
+            // nop if something's terribly wrong!
+            return S_OK;
+
+        // Get the new data into the buffer
+        HRESULT res = video_frame->GetBytes(
+            reinterpret_cast<void **>(&_video_buffer)
         );
-    if (_video_buffer == nullptr) // something's terribly wrong!
-        // nop if something's terribly wrong!
-        return S_OK;
+        // If data could not be read into the buffer, return
+        if (FAILED(res))
+            return res;
+        // Set video frame specs according to new data
+        _video_buffer_length = n_bytes;
+        _cols = video_frame->GetWidth();
+        _rows = video_frame->GetHeight();
+
+        // Propagate new video frame to observers
+        _buffer_video_frame.init_from_specs(
+            _video_buffer, _video_buffer_length, _cols, _rows
+        );
+    }
 
-    // Get the new data into the buffer
-    HRESULT res = video_frame->GetBytes(
-        reinterpret_cast<void **>(&_video_buffer)
-    );
-    // If data could not be read into the buffer, return
-    if (FAILED(res))
-        return res;
-    // Set video frame specs according to new data
-    _video_buffer_length = n_bytes;
-    _cols = video_frame->GetWidth();
-    _rows = video_frame->GetHeight();
-
-    // Propagate new video frame to observers
-    _buffer_video_frame.init_from_specs(
-        _video_buffer, _video_buffer_length, _cols, _rows
-    );
     this->notify(_buffer_video_frame);
 
     // Everything went fine, return success

src/tests/videosourcefactory/leak_checker.cpp

@@ -41,7 +41,16 @@ int main(int argc, char *argv[])
         show_error(argc, argv);
 
     VideoSourceFactory &factory = VideoSourceFactory::get_instance();
-    IVideoSource *source = factory.get_device(device, colour);
+    try
+    {
+        IVideoSource *source = factory.get_device(device, colour);
+    }
+    catch (gg::DeviceOffline &device_offline)
+    {
+        printf("Device offline\n");
+        return EXIT_FAILURE;
+    }
+    printf("OK\n");
 
     /* Upon exiting, the factory should free all
      * allocated memory. So there should be no

src/tests/videosourcefactory/repeat-leak-checker.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+
+ulimit -c unlimited
+
+device=$1
+colour=$2
+num_reps=$3
+for i in `seq 1 $num_reps`; do
+    printf "${i}. run: "
+    ./tests/videosourcefactory/video_source_factory_leak_checker $device $colour
+    sleep 5
+done
+
+ulimit -c 0