Skip to content

Commit

Permalink
Fix Windows crash on exit with upcoming NDI video output plugin (#510)
Browse files Browse the repository at this point in the history
### Fix Windows crash on exit with upcoming NDI video output plugin

### Linked issues
NA

### Summarize your change.

This commit makes sure that the allocated video modules are released
when RV exits.
To do so, the TwkApp::VideoModules container which previously contained
raw pointers (std::vector<VideoModule*>) which were never released on
exit, has been converted to std::vector<std::shared_ptr<VideoModule>>

### Describe the reason for the change.

This missing TwkApp::VideoModules cleanup on exit, was causing a crash
on Windows when the upcoming NDI video output plugin was unloaded.
More specifically: The NDI crash was caused when trying to unload the
NDI dll without having called the NDIlib_destroy().
Note that NDIlib_initialize() is called when allocating an NDI video
module while the NDIlib_destroy() is called when releasing the NDI video
module. So it became essential to fix this TwkApp::VideoModules cleanup
on exit to prevent a crash on Windows.

### Describe what you have tested and on which operating system.
Successfully tested on macOS and Windows.

### Add a list of changes, and note any that might need special
attention during the review.

### If possible, provide screenshots.

Signed-off-by: Bernard Laberge <[email protected]>
  • Loading branch information
bernie-laberge authored Jul 11, 2024
1 parent 0360669 commit ccec94c
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/lib/app/RvCommon/RvApplication.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1787,7 +1787,7 @@ RvApplication::findPresentationDevice(const std::string& dpath) const

if (mindex >= 0)
{
VideoModule* m = videoModules()[mindex];
VideoModule* m = videoModules()[mindex].get();
openVideoModule(m);
const VideoModule::VideoDevices& devs = m->devices();
string dname = parts[1].toUtf8().constData();
Expand Down
8 changes: 4 additions & 4 deletions src/lib/app/RvCommon/RvPreferences.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3249,7 +3249,7 @@ RvPreferences::updateVideo()

if (m_currentVideoDevice < 0 && m_currentVideoModule >= 0)
{
VideoModule* m = vmods[m_currentVideoModule];
const auto m = vmods[m_currentVideoModule];
const VideoModule::VideoDevices& devs = m->devices();
string dname = parts[1].toUtf8().constData();

Expand Down Expand Up @@ -3277,12 +3277,12 @@ RvPreferences::updateVideo()

for (size_t i = 0; i < vmods.size(); i++)
{
TwkApp::VideoModule* mod = vmods[i];
auto mod = vmods[i];
m_ui.videoModuleCombo->addItem(QString::fromUtf8(mod->name().c_str()));

if (i == m_currentVideoModule)
{
if (!mod->isOpen()) RvApp()->openVideoModule(mod);
if (!mod->isOpen()) RvApp()->openVideoModule(mod.get());
m_ui.videoModuleCombo->setCurrentIndex(i);
m_ui.videoDeviceCombo->clear();
m_ui.videoDeviceCombo->hide();
Expand Down Expand Up @@ -3810,7 +3810,7 @@ RvPreferences::currentVideoDevice() const
m_currentVideoModule == -1) return 0;

const IPCore::Application::VideoModules& vmods = RvApp()->videoModules();
VideoModule* mod = vmods[m_currentVideoModule];
const auto mod = vmods[m_currentVideoModule];
const VideoModule::VideoDevices& devs = mod->devices();
return devs[m_currentVideoDevice];
}
Expand Down
23 changes: 4 additions & 19 deletions src/lib/app/RvCommon/RvTopViewToolBar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -485,20 +485,12 @@ RvTopViewToolBar::receiveEvent(const Event& event)
const string deviceName = contents.substr(p+1);
const string moduleName = contents.substr(0, p);

const IPCore::Application::VideoModules& modules = RvApp()->videoModules();

for (size_t i = 0; i < modules.size(); i++)
for (const auto module : RvApp()->videoModules())
{
const VideoModule* module = modules[i];

if (module->name() == moduleName)
{
const VideoModule::VideoDevices& devices = module->devices();

for (size_t q = 0; q < devices.size(); q++)
for (const auto device : module->devices())
{
const VideoDevice* device = devices[q];

if (device->name() == deviceName)
{
m_outputDevice = device;
Expand Down Expand Up @@ -1025,17 +1017,10 @@ RvTopViewToolBar::monitorMenuUpdate()

QString html = "<style type=\"text/css\"> td { padding: 4px; } </style><table>";

const IPCore::Application::VideoModules& modules = RvApp()->videoModules();

for (size_t i = 0; i < modules.size(); i++)
for (const auto module : RvApp()->videoModules())
{
const VideoModule* module = modules[i];
const VideoModule::VideoDevices& devices = module->devices();

for (size_t q = 0; q < devices.size(); q++)
for (const auto device : module->devices())
{
const VideoDevice* device = devices[q];

VideoDevice::VideoFormat format = device->videoFormatAtIndex(device->currentVideoFormat());
VideoDevice::DataFormat data = device->dataFormatAtIndex(device->currentDataFormat());

Expand Down
5 changes: 4 additions & 1 deletion src/lib/app/TwkApp/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ Application::Application() : TwkUtil::Notifier()
Application::~Application()
{
m_app = 0;

// Make sure to release all the video modules before we unload the plugins
m_videoModules.clear();
}

void
Expand Down Expand Up @@ -58,7 +61,7 @@ Application::receive(Notifier* originator,
VideoModule*
Application::primaryVideoModule() const
{
return m_videoModules.empty() ? 0 : m_videoModules.front();
return m_videoModules.empty() ? 0 : m_videoModules.front().get();
}

void
Expand Down
6 changes: 3 additions & 3 deletions src/lib/app/TwkApp/TwkApp/Application.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
#include <TwkUtil/Notifier.h>
#include <TwkApp/Document.h>
#include <TwkApp/OutputPlugins.h>
#include <TwkApp/VideoModule.h>

namespace TwkApp {
class VideoModule;

//
// class Application
Expand All @@ -38,7 +38,7 @@ class Application : public TwkUtil::Notifier
//

typedef std::vector<Document*> Documents;
typedef std::vector<VideoModule*> VideoModules;
typedef std::vector<std::shared_ptr<VideoModule>> VideoModules;
typedef TwkUtil::Notifier Notifier;

//
Expand All @@ -59,7 +59,7 @@ class Application : public TwkUtil::Notifier

void loadOutputPlugins(const std::string& envvar);
void unloadOutputPlugins();
void addVideoModule(VideoModule* m) { m_videoModules.push_back(m); }
void addVideoModule(VideoModule* m) { m_videoModules.push_back(std::shared_ptr<VideoModule>(m)); }
virtual VideoModule* primaryVideoModule() const;
const VideoModules& videoModules() const { return m_videoModules; }

Expand Down
2 changes: 1 addition & 1 deletion src/lib/ip/IPCore/IPCore/Application.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Application : public TwkApp::Application
typedef std::vector<std::string> StringVector;
typedef std::map<std::string,std::string> PropEnvMap;
typedef TwkApp::VideoModule VideoModule;
typedef std::vector<VideoModule*> VideoModules;
typedef std::vector<std::shared_ptr<VideoModule>> VideoModules;
typedef size_t DispatchID;
typedef std::function<void(DispatchID)> DispatchCallback;

Expand Down
2 changes: 1 addition & 1 deletion src/lib/ip/IPCore/IPCore/IPGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class IPGraph : public TwkApp::EventNode
typedef IPNode::IPNodes IPNodes;
typedef TwkApp::VideoModule VideoModule;
typedef TwkApp::VideoDevice VideoDevice;
typedef std::vector<VideoModule*> VideoModules;
typedef std::vector<std::shared_ptr<VideoModule>> VideoModules;
typedef std::vector<DisplayGroupIPNode*> DisplayGroups;
typedef IPImage::InternalGLMatricesContext InternalGLMatricesContext;
typedef int WorkItemID;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/ip/IPCore/IPGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ IPGraph::setPhysicalDevicesInternal(const VideoModules& modules)
{
for (size_t i = 0; i < modules.size(); i++)
{
const VideoModule* module = modules[i];
const VideoModule* module = modules[i].get();
const TwkApp::VideoModule::VideoDevices& devices = module->devices();

for (size_t j = 0; j < devices.size(); j++)
Expand Down

0 comments on commit ccec94c

Please sign in to comment.