From e031bada7d0a474787411f05b6ce7a1b74d0b5e4 Mon Sep 17 00:00:00 2001 From: Maksim Shabunin Date: Fri, 27 Jul 2018 18:25:55 +0300 Subject: [PATCH 1/2] Fixed several issues found by static analysis, Windows-specific --- modules/highgui/src/window_w32.cpp | 14 +++++----- modules/videoio/src/cap_dshow.cpp | 41 ++++++++---------------------- modules/videoio/src/cap_msmf.cpp | 16 +++++++++--- modules/videoio/src/cap_vfw.cpp | 2 +- 4 files changed, 33 insertions(+), 40 deletions(-) diff --git a/modules/highgui/src/window_w32.cpp b/modules/highgui/src/window_w32.cpp index ea7b461f90..945b2e6e78 100644 --- a/modules/highgui/src/window_w32.cpp +++ b/modules/highgui/src/window_w32.cpp @@ -307,8 +307,8 @@ icvLoadWindowPos( const char* name, CvRect& rect ) { HKEY hkey; char szKey[1024]; - strcpy( szKey, icvWindowPosRootKey ); - strcat( szKey, name ); + strcpy_s( szKey, 1024, icvWindowPosRootKey ); + strcat_s( szKey, 1024, name ); rect.x = rect.y = CW_USEDEFAULT; rect.width = rect.height = 320; @@ -368,8 +368,8 @@ icvSaveWindowPos( const char* name, CvRect rect ) HKEY hkey; char szKey[1024]; char rootKey[1024]; - strcpy( szKey, icvWindowPosRootKey ); - strcat( szKey, name ); + strcpy_s( szKey, 1024, icvWindowPosRootKey ); + strcat_s( szKey, 1024, name ); if( RegOpenKeyEx( HKEY_CURRENT_USER,szKey,0,KEY_READ,&hkey) != ERROR_SUCCESS ) { @@ -379,7 +379,7 @@ icvSaveWindowPos( const char* name, CvRect rect ) char oldestKey[1024]; char currentKey[1024]; - strcpy( rootKey, icvWindowPosRootKey ); + strcpy_s( rootKey, 1024, icvWindowPosRootKey ); rootKey[strlen(rootKey)-1] = '\0'; if( RegCreateKeyEx(HKEY_CURRENT_USER, rootKey, 0, NULL, REG_OPTION_NON_VOLATILE, KEY_READ+KEY_WRITE, 0, &hroot, NULL) != ERROR_SUCCESS ) //RegOpenKeyEx( HKEY_CURRENT_USER,rootKey,0,KEY_READ,&hroot) != ERROR_SUCCESS ) @@ -398,7 +398,7 @@ icvSaveWindowPos( const char* name, CvRect rect ) oldestTime.dwLowDateTime > accesstime.dwLowDateTime) ) { oldestTime = accesstime; - strcpy( oldestKey, currentKey ); + strcpy_s( oldestKey, 1024, currentKey ); } } @@ -1500,6 +1500,8 @@ MainWindowProc( HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam ) rgn = CreateRectRgn(0, 0, wrc.right, wrc.bottom); rgn1 = CreateRectRgn(cr.left, cr.top, cr.right, cr.bottom); rgn2 = CreateRectRgn(tr.left, tr.top, tr.right, tr.bottom); + CV_Assert(rgn != 0, rgn1 != 0, rgn2 != 0); + ret = CombineRgn(rgn, rgn, rgn1, RGN_DIFF); ret = CombineRgn(rgn, rgn, rgn2, RGN_DIFF); diff --git a/modules/videoio/src/cap_dshow.cpp b/modules/videoio/src/cap_dshow.cpp index 3a92a81d49..03cb5a4f37 100644 --- a/modules/videoio/src/cap_dshow.cpp +++ b/modules/videoio/src/cap_dshow.cpp @@ -811,6 +811,8 @@ void videoDevice::NukeDownstream(IBaseFilter *pBF){ IEnumPins *pins = NULL; PIN_INFO pininfo; HRESULT hr = pBF->EnumPins(&pins); + if (hr != S_OK || !pins) + return; pins->Reset(); while (hr == NOERROR) { @@ -838,7 +840,7 @@ void videoDevice::NukeDownstream(IBaseFilter *pBF){ pP->Release(); } } - if (pins) pins->Release(); + pins->Release(); } @@ -999,17 +1001,6 @@ videoDevice::~videoDevice(){ (pGraph) = 0; } - //delete our pointers - delete pDestFilter; - delete pVideoInputFilter; - delete pGrabberF; - delete pGrabber; - delete pControl; - delete streamConf; - delete pMediaEvent; - delete pCaptureGraph; - delete pGraph; - DebugPrintOut("SETUP: Device %i disconnected and freed\n\n",myID); } @@ -1654,7 +1645,7 @@ bool videoInput::getVideoSettingFilter(int deviceID, long Property, long &min, l IAMVideoProcAmp *pAMVideoProcAmp = NULL; hr = VD->pVideoInputFilter->QueryInterface(IID_IAMVideoProcAmp, (void**)&pAMVideoProcAmp); - if(FAILED(hr)){ + if(FAILED(hr) || !pAMVideoProcAmp){ DebugPrintOut("setVideoSetting - QueryInterface Error\n"); #if 0 if(VD->pVideoInputFilter)VD->pVideoInputFilter->Release(); @@ -1676,7 +1667,7 @@ bool videoInput::getVideoSettingFilter(int deviceID, long Property, long &min, l hr = pAMVideoProcAmp->Get(Property, ¤tValue, &flags); } - if(pAMVideoProcAmp)pAMVideoProcAmp->Release(); + pAMVideoProcAmp->Release(); #if 0 if(VD->pVideoInputFilter)VD->pVideoInputFilter->Release(); if(VD->pVideoInputFilter)VD->pVideoInputFilter = NULL; @@ -1881,7 +1872,7 @@ bool videoInput::getVideoSettingCamera(int deviceID, long Property, long &min, l IAMCameraControl *pIAMCameraControl = NULL; hr = VD->pVideoInputFilter->QueryInterface(IID_IAMCameraControl, (void**)&pIAMCameraControl); - if(FAILED(hr)){ + if(FAILED(hr) || !pIAMCameraControl){ DebugPrintOut("setVideoSetting - QueryInterface Error\n"); #if 0 if(VD->pVideoInputFilter)VD->pVideoInputFilter->Release(); @@ -1902,7 +1893,7 @@ bool videoInput::getVideoSettingCamera(int deviceID, long Property, long &min, l hr = pIAMCameraControl->Get(Property, ¤tValue, &flags); } - if(pIAMCameraControl)pIAMCameraControl->Release(); + pIAMCameraControl->Release(); #if 0 if(VD->pVideoInputFilter)VD->pVideoInputFilter->Release(); if(VD->pVideoInputFilter)VD->pVideoInputFilter = NULL; @@ -2595,7 +2586,7 @@ int videoInput::start(int deviceID, videoDevice *VD){ //we do this because webcams don't have a preview mode hr = VD->pCaptureGraph->FindInterface(&CAPTURE_MODE, &MEDIATYPE_Video, VD->pVideoInputFilter, IID_IAMStreamConfig, (void **)&VD->streamConf); - if(FAILED(hr)){ + if(FAILED(hr) || !VD->streamConf){ DebugPrintOut("ERROR: Couldn't config the stream!\n"); stopDevice(deviceID); return hr; @@ -2737,14 +2728,8 @@ int videoInput::start(int deviceID, videoDevice *VD){ //lets try freeing our stream conf here too //this will fail if the device is already running - if(VD->streamConf){ - VD->streamConf->Release(); - VD->streamConf = NULL; - }else{ - DebugPrintOut("ERROR: connecting device - prehaps it is already being used?\n"); - stopDevice(deviceID); - return S_FALSE; - } + VD->streamConf->Release(); + VD->streamConf = NULL; //NULL RENDERER// @@ -3093,7 +3078,7 @@ HRESULT videoInput::routeCrossbar(ICaptureGraphBuilder2 **ppBuild, IBaseFilter * IAMCrossbar *pXBar1 = NULL; HRESULT hr = pBuild->FindInterface(&LOOK_UPSTREAM_ONLY, NULL, pVidFilter, IID_IAMCrossbar, (void**)&pXBar1); - if (SUCCEEDED(hr)) + if (SUCCEEDED(hr) && pXBar1) { bool foundDevice = false; @@ -3163,10 +3148,6 @@ HRESULT videoInput::routeCrossbar(ICaptureGraphBuilder2 **ppBuild, IBaseFilter * //we were getting a crash otherwise //if(Crossbar)Crossbar->Release(); //if(Crossbar)Crossbar = NULL; - - if(pXBar1)pXBar1->Release(); - if(pXBar1)pXBar1 = NULL; - }else{ DebugPrintOut("SETUP: You are a webcam or snazzy firewire cam! No Crossbar needed\n"); return hr; diff --git a/modules/videoio/src/cap_msmf.cpp b/modules/videoio/src/cap_msmf.cpp index 35043ee535..863f46bc89 100644 --- a/modules/videoio/src/cap_msmf.cpp +++ b/modules/videoio/src/cap_msmf.cpp @@ -91,7 +91,7 @@ static bool pMFCreateDXGIDeviceManager_initialized = false; static FN_MFCreateDXGIDeviceManager pMFCreateDXGIDeviceManager = NULL; static void init_MFCreateDXGIDeviceManager() { - HMODULE h = LoadLibraryA("mfplat.dll"); + HMODULE h = LoadLibraryExA("mfplat.dll", NULL, LOAD_LIBRARY_SEARCH_SYSTEM32); if (h) { pMFCreateDXGIDeviceManager = (FN_MFCreateDXGIDeviceManager)GetProcAddress(h, "MFCreateDXGIDeviceManager"); @@ -1720,7 +1720,7 @@ bool CvCapture_MSMF::setProperty( int property_id, double value ) return setTime(duration * value, true); break; case CV_CAP_PROP_POS_FRAMES: - if (getFramerate(nativeFormat) != 0) + if (std::fabs(getFramerate(nativeFormat)) > 0) return setTime(value * 1e7 / getFramerate(nativeFormat), false); break; case CV_CAP_PROP_POS_MSEC: @@ -1978,7 +1978,17 @@ private: CvVideoWriter_MSMF::CvVideoWriter_MSMF(): MF(Media_Foundation::getInstance()), - initiated(false) + videoWidth(0), + videoHeight(0), + fps(0), + bitRate(0), + frameSize(0), + encodingFormat(), + inputFormat(), + streamIndex(0), + initiated(false), + rtStart(0), + rtDuration(0) { } diff --git a/modules/videoio/src/cap_vfw.cpp b/modules/videoio/src/cap_vfw.cpp index 0d71a0c2a5..f62baf4e71 100644 --- a/modules/videoio/src/cap_vfw.cpp +++ b/modules/videoio/src/cap_vfw.cpp @@ -377,8 +377,8 @@ LRESULT PASCAL CvCaptureCAM_VFW::frameCallback( HWND hWnd, VIDEOHDR* hdr ) if (!hWnd) return FALSE; capture = (CvCaptureCAM_VFW*)capGetUserData(hWnd); + if (!capture) return (LRESULT)FALSE; capture->hdr = hdr; - return (LRESULT)TRUE; } From dd8e99045144b4828a79eba5a1e592fc489927c0 Mon Sep 17 00:00:00 2001 From: Maksim Shabunin Date: Fri, 27 Jul 2018 18:41:39 +0300 Subject: [PATCH 2/2] Fixed several issues found by static analysis, GStreamer backend --- modules/videoio/src/cap_gstreamer.cpp | 37 ++++++++++----------------- 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/modules/videoio/src/cap_gstreamer.cpp b/modules/videoio/src/cap_gstreamer.cpp index c3100f56f3..8758b21dd9 100644 --- a/modules/videoio/src/cap_gstreamer.cpp +++ b/modules/videoio/src/cap_gstreamer.cpp @@ -1224,7 +1224,11 @@ Ptr cv::createGStreamerCapture(int index) class CvVideoWriter_GStreamer : public CvVideoWriter { public: - CvVideoWriter_GStreamer() { init(); } + CvVideoWriter_GStreamer() + : pipeline(0), source(0), encodebin(0), file(0), buffer(0), input_pix_fmt(0), + num_frames(0), framerate(0) + { + } virtual ~CvVideoWriter_GStreamer() CV_OVERRIDE { close(); } virtual bool open( const char* filename, int fourcc, @@ -1232,7 +1236,6 @@ public: virtual void close(); virtual bool writeFrame( const IplImage* image ) CV_OVERRIDE; protected: - void init(); const char* filenameToMimetype(const char* filename); GstElement* pipeline; GstElement* source; @@ -1245,22 +1248,6 @@ protected: double framerate; }; -/*! - * \brief CvVideoWriter_GStreamer::init - * initialise all variables - */ -void CvVideoWriter_GStreamer::init() -{ - pipeline = NULL; - source = NULL; - encodebin = NULL; - file = NULL; - buffer = NULL; - - num_frames = 0; - framerate = 0; -} - /*! * \brief CvVideoWriter_GStreamer::close * ends the pipeline by sending EOS and destroys the pipeline and all @@ -1282,17 +1269,19 @@ void CvVideoWriter_GStreamer::close() //wait for EOS to trickle down the pipeline. This will let all elements finish properly GstBus* bus = gst_element_get_bus(pipeline); GstMessage *msg = gst_bus_timed_pop_filtered(bus, GST_CLOCK_TIME_NONE, (GstMessageType)(GST_MESSAGE_ERROR | GST_MESSAGE_EOS)); - if (GST_MESSAGE_TYPE(msg) == GST_MESSAGE_ERROR) + if (!msg || GST_MESSAGE_TYPE(msg) == GST_MESSAGE_ERROR) { CV_WARN("Error during VideoWriter finalization\n"); + if(msg != NULL) + { + gst_message_unref(msg); + g_object_unref(G_OBJECT(bus)); + } return; } - if(msg != NULL) - { - gst_message_unref(msg); - g_object_unref(G_OBJECT(bus)); - } + gst_message_unref(msg); + g_object_unref(G_OBJECT(bus)); status = gst_element_set_state (pipeline, GST_STATE_NULL); if (status == GST_STATE_CHANGE_ASYNC)