After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 727306 - decklink: fails to initialize on cards that only have input or only output interfaces
decklink: fails to initialize on cards that only have input or only output in...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-03-29 17:30 UTC by Isaac Smith
Modified: 2014-06-19 20:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.14 KB, patch)
2014-03-29 21:31 UTC, Isaac Smith
none Details | Review
Fix support for HW without output (2.72 KB, patch)
2014-03-29 22:42 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Isaac Smith 2014-03-29 17:30:28 UTC
When initializing devices in decklink.cpp, for some reason the code would return immediately if it can't get an input or output interface on the card. While this isn't a problem for most Blackmagic cards, two models only have an input or an output interface, causing certain interfaces not to be grabbed. This fixes bug #725175 as well. 

diff --git a/sys/decklink/gstdecklink.cpp b/sys/decklink/gstdecklink.cpp
index 848ff28..067665e 100644
--- a/sys/decklink/gstdecklink.cpp
+++ b/sys/decklink/gstdecklink.cpp
@@ -228,21 +228,18 @@ init_devices (void)
         (void **) &devices[i].input);
     if (ret != S_OK) {
       GST_WARNING ("selected device does not have input interface");
-      return;
     }
 
     ret = decklink->QueryInterface (IID_IDeckLinkOutput,
         (void **) &devices[i].output);
     if (ret != S_OK) {
       GST_WARNING ("selected device does not have output interface");
-      return;
     }
 
     ret = decklink->QueryInterface (IID_IDeckLinkConfiguration,
         (void **) &devices[i].config);
     if (ret != S_OK) {
       GST_WARNING ("selected device does not have config interface");
-      return;
     }
 
     ret = iterator->Next (&decklink);
Comment 1 Nicolas Dufresne (ndufresne) 2014-03-29 19:36:38 UTC
That's seems fair, but incomplete. decklinksrc seems to care about input and config, decklinksink only cares about output (seems twisted, but I'm not surprised after having worked with V4L2).

The reason it's incomplete, is that both sink and src assume these will be present, calling a function on the returned structure, leading to crash. To make it worst, this structure is not initialized from what I see (though this is C++, I might be wrong).

So I would say, unless C++ make this safe, memset to 0 the array of Devices, and add required NULL check / failure in src and sink element. This way it can cleanly fail is the element is setup wrongly. Also, when done, commit your changes with your real name and email address into you local git repo (git master), and use git format-patch -1 to create a file we can merge using git am. We can deal with the backporting to 1.2, this element havent changed much.

Thanks for your time and contribution.
Comment 2 Nicolas Dufresne (ndufresne) 2014-03-29 19:39:46 UTC
Oh, when using git format-patch -1, it will create a file, that you should attach to this bug, not paste in a comment (which cause line wrapping). Thanks again, I'm sure many owner of these card will appreciate your effort, though i don't  own one myself, so make sure you test all combination you can think of, as I only have my eyes to detect errors here.
Comment 3 Isaac Smith 2014-03-29 21:02:38 UTC
I'm not a C/C++ programmer, so figuring out how to properly restructure the program is beyond me. I'd be happy to test any changes, however, just let me know. I've tested the changes with my two Decklink cards, but neither of them support outputting (or sinking in GST terms) video, only inputting/sourcing. 

I'll try and figure out git format-patch too, I'm pretty new to git and version control in general.
Comment 4 Isaac Smith 2014-03-29 21:31:15 UTC
Created attachment 273251 [details] [review]
Patch
Comment 5 Nicolas Dufresne (ndufresne) 2014-03-29 22:42:44 UTC
Created attachment 273257 [details] [review]
Fix support for HW without output

Please test this patch. In order to test this, I may suggest these quick tests:

/* Should fail, not crash */
gst-launch-1.0 videotestsrc ! decklinksink device-number=0
gst-launch-1.0 videotestsrc ! decklinksink device-number=0

/* Should work ? */
gst-launch-1.0 decklinksrc device-number=0 ! videoconvert ! xvimagesink
gst-launch-1.0 decklinksrc device-number=1 ! videoconvert ! xvimagesink

/* Should fail, not crash */
gst-launch-1.0 decklinksrc device-number=2 ! videoconvert ! xvimagesink
Comment 6 Nicolas Dufresne (ndufresne) 2014-03-29 22:43:43 UTC
Btw, to apply, remove your patch (git reset --hard <commit hash before>) and apply, git am patch.
Comment 7 Nicolas Dufresne (ndufresne) 2014-04-23 17:23:00 UTC
commit 1c1cc73a3b230454663971656515297f9ee8bd9f
Author: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Date:   Sat Mar 29 18:34:26 2014 -0400

    decklink: Fix support for HW without output
    
    Devices suitable for decklinksrc may not have any output, hence querying
    the input returns NULL. Add support for all cases where
    input/output/config may be missing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727306
Comment 8 Ed T 2014-06-19 19:57:15 UTC
I have a deckink minirecorder card and an decklink I/O card.  The recorder card (with input only) still segfaults sometimes with version 1.3.2.  The I/O card behaves normally.  Has this patch been applied to 1.3.2?  If so, it seems not to do the trick.
Comment 9 Tim-Philipp Müller 2014-06-19 20:04:54 UTC
Yes, it should be in 1.3.2.

Could you please file a new bug (or clone this bug) and get a stack trace of the crash?