Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove debugging code from DirectShow video capture #9051

Closed
wants to merge 2 commits into from

Conversation

zhmu
Copy link
Contributor

@zhmu zhmu commented Jun 30, 2017

This code is impossible to enable or disable at runtime, and that wouldn't be very useful either due to the static VideoCapture_DShow::g_VI declaration.

As this code is unavailable in release mode anyway and only seemed to have served purpose when creating this code (it was present in the original version in
github ofTheo/videoInput), I suggest it be removed.

resolves #8926

This pullrequest changes

This code removes the debugging code from the DirectShow video input, which is always enabled in debugging mode. As the code has been remarkably stable for the last 6 years, I hope we can do without it.

This code is impossible to enable or disable at runtime, and that wouldn't be very useful either due to the static VideoCapture_DShow::g_VI declaration.

As this code is unavailable in release mode anyway and only seemed to have served purpose when creating this code (it was present in the original version in
github ofTheo/videoInput), I suggest it be removed.
@alalek
Copy link
Member

alalek commented Jun 30, 2017

This debug code doesn't harm anything (and completely disabled in Release), so I believe we can keep it here.

@alalek
Copy link
Member

alalek commented Jun 30, 2017

remarkably stable for the last 6 years

This is not a criteria for code removal.
There are several bugs filled for "DShow", so ability to enable this debug code is useful for future improvements/fixes/refactoring.

@zhmu
Copy link
Contributor Author

zhmu commented Jun 30, 2017

Perhaps that is not a criteria, but as the linked issue remarks: having this code adds debugging output (the ***** VIDEOINPUT LIBRARY - 0.1995 - TFW07 ***** startup banner) to debug builds. This breaks things like attempting to run unit tests from Resharper C/C++, which is extremely annoying (especially as you often do that with debug builds)

Would you settle for a change which only removes that specific line from the output?

This is at least useful when using an SVM one-class linear classifier, so there are valid use cases.
@alalek
Copy link
Member

alalek commented Jun 30, 2017

Perhaps this should work:

-static bool gs_verbose = true;
+static bool gs_verbose = false;

 static void DebugPrintOut(const char *format, ...)
 {
+    static bool force_debug = getenv("OPENCV_DSHOW_DEBUG") ? true : false;
-    if (gs_verbose)
+    if (gs_verbose || force_debug)
     {

@zhmu
Copy link
Contributor Author

zhmu commented Jun 30, 2017

That would work, but there is no way to change gs_verbose as you cannot call videoInput::setVerbose() from outside cap_dshow.cpp (and even if you could, there is still the static VideoCapture_DShow::g_VI to worry about)

Making these messages opt-in with an environment variable or similar would definitely have my support. I'd say you can just get rid of the gs_verbose variable completely.

@alalek
Copy link
Member

alalek commented Jun 30, 2017

I agree, that setVerbose is internal API only and it is not exposed into public OpenCV API - so there is no access to it.

static VideoCapture_DShow::g_VI

Using of this global variable is not a good practice. This should be definitely refactored somehow.

@zhmu
Copy link
Contributor Author

zhmu commented Jul 13, 2017

Let's close this - I will submit a new pull request shortly.

@zhmu zhmu closed this Jul 13, 2017
zhmu added a commit to zhmu/opencv that referenced this pull request Jul 13, 2017
…HOW_DEBUG is explicitly set to non-zero

Based on discussion at: opencv#9051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'***** VIDEOINPUT LIBRARY - 0.1995 - TFW07 *****' message displayed during startup in debug mode
2 participants