VideoCapture/DSHOW : Allow to set CAP_PROP_CONVERT_RGB before FOURCC/FPS/CHANNEL/WIDTH/HEIGHT. by OlivierLDff · Pull Request #19370 · opencv/opencv

OlivierLDff · 2021-01-22T08:31:51Z

Related issue : #19367

The PR fixed it the case to set FOURCC/FPS/CHANNEL & CONVERT_RGB in any order.
Every time stopDevice is called, setConvertRgb should be called again with correct value.
For example settings FPS after CONVERT_RGB:

opencv/modules/videoio/src/cap_dshow.cpp

Lines 3455 to 3467 in ea41f89

    
           case CV_CAP_PROP_FPS: 
        
           { 
        
               int fps = cvRound(propVal); 
        
               if (fps != g_VI.getFPS(m_index)) 
        
               { 
        
                   g_VI.stopDevice(m_index); 
        
                   g_VI.setIdealFramerate(m_index, fps); 
        
                   if (m_widthSet > 0 && m_heightSet > 0) 
        
                       g_VI.setupDevice(m_index, m_widthSet, m_heightSet); 
        
                   else 
        
                       g_VI.setupDevice(m_index); 
        
               } 
        
               return g_VI.isDeviceSetup(m_index);

Same for CHANNEL

opencv/modules/videoio/src/cap_dshow.cpp

Lines 3447 to 3453 in ea41f89

    
           case CAP_PROP_CHANNEL: 
        
               if (cvFloor(propVal) < 0) 
        
                   break; 
        
               g_VI.stopDevice(m_index); 
        
               g_VI.setupDevice(m_index,  cvFloor(propVal)); 
        
               break;

And when settings FOURCC/WIDTH/HEIGHT

opencv/modules/videoio/src/cap_dshow.cpp

Lines 3494 to 3500 in ea41f89

    
           if (m_width != g_VI.getWidth(m_index) || m_height != g_VI.getHeight(m_index) || m_fourcc != g_VI.getFourcc(m_index) ) 
        
           { 
        
               int fps = static_cast<int>(g_VI.getFPS(m_index)); 
        
               g_VI.stopDevice(m_index); 
        
               g_VI.setIdealFramerate(m_index, fps); 
        
               g_VI.setupDeviceFourcc(m_index, m_width, m_height, m_fourcc); 
        
           }

Detailed description

I'm using Boson camera from FLIR. I'm interested in Y16 format video. I'm using DSHOW backend on windows.
I noticed that setting CAP_PROP_CONVERT_RGB then something like CAP_PROP_FOURCC will revert CAP_PROP_CONVERT_RGB to the default value.

Steps to reproduce

#include <opencv2/videoio.hpp>

// ...

cv::VideoCapture cap;
cap.open(0, cv::CAP_DSHOW);
f(!cap.isOpened())
    return;

// Print true, that is ok, default expected behavior.
std::cout << "CAP_PROP_CONVERT_RGB: " << cap.get(cv::CAP_PROP_CONVERT_RGB) << std::endl; 

cap.set(cv::CAP_PROP_CONVERT_RGB, false);
cap.set(cv::CAP_PROP_FOURCC, cv::VideoWriter::fourcc('Y', '1', '6', ' '));

// Print true, should print false.
std::cout << "CAP_PROP_CONVERT_RGB: " << cap.get(cv::CAP_PROP_CONVERT_RGB) << std::endl;

Proposed solution

I fixed it into a fork.
I introduced a m_convertRGBSet boolean value inside VideoCapture_DShow that is updated when CAP_PROP_CONVERT_RGB is set.
Then i call g_VI.setConvertRGB when g_VI get reinitialized with the saved value.

Current workaround

I believe what everyone is doing right now is first set CAP_PROP_FOURCC then CAP_PROP_CONVERT_RGB.
Especially boson user, like it is shown on FLIR website

I believe it should work in both order. Or it should be documented somewhere.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

I agree to contribute to the project under Apache 2 License.
To the best of my knowledge, the proposed patch is not based on a code under GPL or other license that is incompatible with OpenCV
The PR is proposed to proper branch
There is reference to original bug report and related work
There is accuracy test, performance test and test data in opencv_extra repository, if applicable
Patch to opencv_extra has the same branch name.
The feature is well documented and sample code can be built with the project CMake

force_builder=Win32

OlivierLDff · 2021-01-22T09:18:21Z

I pushed a new commit OlivierLDff@2b415a3 that should fix every case.

Something like that now works:

cap.set(cv::CAP_PROP_CONVERT_RGB, false);
cap.set(cv::CAP_PROP_FPS, 60);
cap.set(cv::CAP_PROP_FRAME_WIDTH, 320 );
cap.set(cv::CAP_PROP_FRAME_HEIGHT, 256);
cap.set(cv::CAP_PROP_FOURCC, cv::VideoWriter::fourcc('Y', '1', '6', ' '));

When setting CAP_PROP_CONVERT_RGB, value is saved into m_convertRGBSet (only if set was a success, ie device is setup).
When device cap.close() the m_convertRGBSet introduced variable is reset to true as expected.
Each time g_VI.stopDevice/g_VI.setupDevice is called, m_convertRGBSet is restore with g_VI.setConvertRGB call.

…NNEL

alalek

Thank you for contribution!

Could you please add your usecase as "disabled" test into this file?

TEST(DISABLED_videoio_camera, dshow_fourcc_y16)
{
    ... initialize "cap" with properties ...
    Mat lastFrame;
    test_readFrames(capture, 100, &lastFrame);
    ASSERT_FALSE(lastFrame.empty());
    EXPECT_EQ(CV_16UC1, lastFrame.type()); // <== change actual value here
}

…olean value on first property set.

OlivierLDff · 2021-01-28T08:33:57Z

Test added b868c39.
Thanks to it i found a bug fixed in ec6c281.
The test run fine on my computer connecting with my internal webcam.

git clone https://github.com/OlivierLDff/opencv
cd opencv
git checkout patch-dshow-convertrgb
mkdir build && cd build
cmake -DBUILD_TESTS=ON ..

# Test Debug
cmake --build . --target opencv_test_videoio --config "Debug"
./bin/Debug/opencv_test_videoiod.exe --gtest_filter=*dshow_convert_rgb_persistency* --gtest_also_run_disabled_tests

# Test Release
cmake --build . --target opencv_test_videoio --config "Release"
./bin/Release/opencv_test_videoio.exe --gtest_filter=*dshow_convert_rgb_persistency* --gtest_also_run_disabled_tests

If you want the test to fail, simply restore

opencv/modules/videoio/src/cap_dshow.cpp

Lines 3482 to 3485 in 1363496

    
           case CV_CAP_PROP_CONVERT_RGB: 
        
           { 
        
               return g_VI.setConvertRGB(m_index, cvRound(propVal) == 1); 
        
           }

OlivierLDff · 2021-01-28T08:53:52Z

I see that OpenCV CN Ubuntu 20.04 arm64 failed due to error: RPC failed; curl 56 GnuTLS recv error (-54): Error in the pull function.
Can you please retrigger CI manually?
Anyway only modification are in cap_dshow which is windows only source files

alalek

Thank you for update!

You can ignore infrastructure (network) issues on China OpenCV CI. "default" is required to be green for now.

alalek · 2021-01-28T10:41:27Z

modules/videoio/test/test_camera.cpp

+{
+    VideoCapture capture(CAP_DSHOW);
+    ASSERT_TRUE(capture.isOpened());
+    ASSERT_TRUE(capture.set(CAP_PROP_CONVERT_RGB, false));


Please avoid build warning here:

ASSERT_TRUE(capture.set(CAP_PROP_CONVERT_RGB, 0/*false*/));

bf6460c Should fix it

913739b should work better

alalek · 2021-01-28T18:31:35Z

modules/videoio/test/test_camera.cpp

+    std::cout << "     height: " << capture.get(CAP_PROP_FRAME_HEIGHT) << std::endl;
+    std::cout << "Capturing FPS: " << capture.get(CAP_PROP_FPS) << std::endl;
+    ASSERT_DOUBLE_EQ(capture.get(CAP_PROP_CONVERT_RGB), 0);
+    capture.release();


Please add lastFrame check (as proposed above).
Look like CAP_PROP_CONVERT_RGB=0 + CAP_PROP_FOURCC=Y16 still doesn't work: .type() is 8UC3, frame is in color.

You can add this for local experiments:

imshow("test", lastFrame); waitKey();

This is not the bug this PR is correcting.
This PR is correcting the fact that CONVERT_RGB once set to false should stay false no matters if width/height/fourcc/fps is set after.
This is what my test is doing. This is what my PR is merging. This is not related to the fact i'm opening my camera in Y16.

If you look implementation, it seems like convertRgb is used only with MEDIASUBTYPE_Y16, so yes it is linked a little bit with Y16.

The fact that CAP_PROP_CONVERT_RGB=0 + CAP_PROP_FOURCC=Y16 doesn't give 16U mat is another bug. If the camera support being opened in Y16 mode, then CV_16UC1 == lastFrame.type()

I tested with FLIR Boson320 i can confirm, the read frame is CV_16UC1.
When opening my webcam with the same code (just changing the id), the result mat is CV_8UC3.

This might be another bug, but this is unrelated to what my PR fix.

The code that doesn't open Y16 if camera doesn't support is there i believe:

opencv/modules/videoio/src/cap_dshow.cpp

Lines 2616 to 2623 in 900051c

hr = VD->streamConf->SetFormat(VD->pAmMediaType);

if(hr == S_OK){

if( tmpType != NULL )MyDeleteMediaType(tmpType);

return true;

}else{

VD->streamConf->SetFormat(tmpType);

if( tmpType != NULL )MyDeleteMediaType(tmpType);

}

From reading source code it also seems that CAP_PROP_CONVERT_RGB=1 + CAP_PROP_FOURCC=Y8 will give you CV_8UC1 as output

opencv/modules/videoio/src/cap_dshow.cpp

Lines 3562 to 3569 in 900051c

// Set suitable output matrix type (e-Con systems)

if (checkSingleByteFormat(g_VI.getMediasubtype(m_index))){

frame.create(Size(w, h), CV_8UC1);

} else if (g_VI.getMediasubtype(m_index) == MEDIASUBTYPE_Y16 && !convertRGB) {

frame.create(Size(w, h), CV_16UC1);

} else {

frame.create(Size(w, h), CV_8UC3);

}

This might be another bug, but this is unrelated to what my PR fix.

OK.

Please fix remaining coding style issue (check "Docs" builder logs) before merge.

I'm not sure to understand, where in Docs builder? I'm not familiar with Jenkins.

"default" entry: http://pullrequest.opencv.org/#/summary/opencv

OlivierLDff · 2021-01-29T09:44:43Z

Can you please retrigger OpenCV CN Ubuntu 18.04 x86-64 ?

alalek · 2021-01-29T10:07:51Z

This build is optional. Problem is infrastructure(network) related. Ignore this.

alalek

Thank you!

VideoCapture/DSHOW : Allow to set CAP_PROP_CONVERT_RGB before FOURCC/FPS/CHANNEL/WIDTH/HEIGHT. * 🐛 cap_dshow : Allow to set CAP_PROP_CONVERT_RGB before FOURCC/FPS/CHANNEL * 🐛 cap_dshow : fix g_VI.setConvertRGB not being called with correct boolean value on first property set. * ✅ cap_dshow : Test CAP_PROP_CONVERT_RGB persistence * 🚨 Fix cast from bool to double * 🚨 Fix trailing whitespace

OlivierLDff mentioned this pull request Jan 22, 2021

videoio : DSHOW CAP_PROP_FOURCC/CAP_PROP_CONVERT_RGB set order doesn't work the same. #19367

Closed

4 tasks

OlivierLDff force-pushed the patch-dshow-convertrgb branch from a6ecf9d to dcfe6c0 Compare January 22, 2021 09:10

OlivierLDff changed the title ~~VideoCapture/DSHOW : Allow to set CAP_PROP_CONVERT_RGB before CAP_PROP_FOURCC.~~ VideoCapture/DSHOW : Allow to set CAP_PROP_CONVERT_RGB before FOURCC/FPS/CHANNEL. Jan 22, 2021

OlivierLDff changed the title ~~VideoCapture/DSHOW : Allow to set CAP_PROP_CONVERT_RGB before FOURCC/FPS/CHANNEL.~~ VideoCapture/DSHOW : Allow to set CAP_PROP_CONVERT_RGB before FOURCC/FPS/CHANNEL/WIDTH/HEIGHT. Jan 22, 2021

🐛 cap_dshow : Allow to set CAP_PROP_CONVERT_RGB before FOURCC/FPS/CHA…

2b415a3

…NNEL

OlivierLDff force-pushed the patch-dshow-convertrgb branch from dcfe6c0 to 2b415a3 Compare January 22, 2021 09:24

alalek reviewed Jan 28, 2021

View reviewed changes

OlivierLDff added 2 commits January 28, 2021 09:30

🐛 cap_dshow : fix g_VI.setConvertRGB not being called with correct bo…

ec6c281

…olean value on first property set.

✅ cap_dshow : Test CAP_PROP_CONVERT_RGB persistence

b868c39

alalek reviewed Jan 28, 2021

View reviewed changes

🚨 Fix cast from bool to double

913739b

OlivierLDff force-pushed the patch-dshow-convertrgb branch from bf6460c to 913739b Compare January 28, 2021 10:58

alalek reviewed Jan 28, 2021

View reviewed changes

alalek mentioned this pull request Jan 29, 2021

Can not set CAP_PROP_CONVERT_RGB to false in VideoCapture When use the Web camera. #11398

Closed

🚨 Fix trailing whitespace

db729e1

alalek approved these changes Jan 29, 2021

View reviewed changes

alalek merged commit 4c7f562 into opencv:master Jan 29, 2021

alalek mentioned this pull request Apr 9, 2021

(5.x) Merge 4.x #19885

Merged

	case CV_CAP_PROP_FPS:
	{
	int fps = cvRound(propVal);
	if (fps != g_VI.getFPS(m_index))
	{
	g_VI.stopDevice(m_index);
	g_VI.setIdealFramerate(m_index, fps);
	if (m_widthSet > 0 && m_heightSet > 0)
	g_VI.setupDevice(m_index, m_widthSet, m_heightSet);
	else
	g_VI.setupDevice(m_index);
	}
	return g_VI.isDeviceSetup(m_index);

	case CAP_PROP_CHANNEL:

	if (cvFloor(propVal) < 0)
	break;
	g_VI.stopDevice(m_index);
	g_VI.setupDevice(m_index, cvFloor(propVal));
	break;

	if (m_width != g_VI.getWidth(m_index) \|\| m_height != g_VI.getHeight(m_index) \|\| m_fourcc != g_VI.getFourcc(m_index) )
	{
	int fps = static_cast<int>(g_VI.getFPS(m_index));
	g_VI.stopDevice(m_index);
	g_VI.setIdealFramerate(m_index, fps);
	g_VI.setupDeviceFourcc(m_index, m_width, m_height, m_fourcc);
	}

	hr = VD->streamConf->SetFormat(VD->pAmMediaType);
	if(hr == S_OK){
	if( tmpType != NULL )MyDeleteMediaType(tmpType);
	return true;
	}else{
	VD->streamConf->SetFormat(tmpType);
	if( tmpType != NULL )MyDeleteMediaType(tmpType);
	}

	// Set suitable output matrix type (e-Con systems)
	if (checkSingleByteFormat(g_VI.getMediasubtype(m_index))){
	frame.create(Size(w, h), CV_8UC1);
	} else if (g_VI.getMediasubtype(m_index) == MEDIASUBTYPE_Y16 && !convertRGB) {
	frame.create(Size(w, h), CV_16UC1);
	} else {
	frame.create(Size(w, h), CV_8UC3);
	}

Uh oh!

Conversation

OlivierLDff commented Jan 22, 2021 • edited by alalek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Detailed description

Steps to reproduce

Proposed solution

Current workaround

Pull Request Readiness Checklist

Uh oh!

OlivierLDff commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

OlivierLDff commented Jan 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OlivierLDff commented Jan 28, 2021

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

alalek Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

OlivierLDff Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

OlivierLDff Jan 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alalek Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

OlivierLDff Jan 28, 2021

Choose a reason for hiding this comment

Uh oh!

alalek Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

OlivierLDff Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

alalek Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

OlivierLDff commented Jan 29, 2021

Uh oh!

alalek commented Jan 29, 2021

Uh oh!

alalek left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

OlivierLDff commented Jan 22, 2021 •

edited by alalek

Loading

OlivierLDff commented Jan 22, 2021 •

edited

Loading

OlivierLDff commented Jan 28, 2021 •

edited

Loading

OlivierLDff Jan 28, 2021 •

edited

Loading