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 736926 - dshowvideosrc: error if set_caps is called while running
dshowvideosrc: error if set_caps is called while running
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-18 20:22 UTC by Joshua M. Doe
Modified: 2014-09-24 07:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
suggested patch to allow set_caps to be called multiple times (3.51 KB, patch)
2014-09-18 20:22 UTC, Joshua M. Doe
needs-work Details | Review
moved decls up and did this for audiosrc as well (5.03 KB, patch)
2014-09-23 20:50 UTC, Joshua M. Doe
committed Details | Review

Description Joshua M. Doe 2014-09-18 20:22:12 UTC
Created attachment 286533 [details] [review]
suggested patch to allow set_caps to be called multiple times

I have a pipeline using dshowvideosrc which calls set_caps while it is already running, resulting in the error "At least one of the pins involved in the operation is already connected", VFW_E_ALREADY_CONNECTED. So it seems simply calling media_filter->Stop() is not enough. This patch checks if the new caps are the same, returning immediately, otherwise it disconnects the pins before reconnecting. Not sure if it's better to reconfigure the existing connection, or if that's even possible.
Comment 1 Jérôme Laheurte 2014-09-19 09:19:46 UTC
Is there a sure way to reproduce this ? Never happened to me... Anyway, the patch looks fine to me, but the same thing should probably be done for the audio capture.
Comment 2 Sebastian Dröge (slomo) 2014-09-23 20:06:51 UTC
Review of attachment 286533 [details] [review]:

Yes, should be done for the audio source too. Looks mostly good though

::: sys/dshowsrcwrapper/gstdshowvideosrc.cpp
@@ +582,3 @@
+  GstCaps *current_caps = gst_pad_get_current_caps (pad);
+  gst_object_unref (pad);
+  gboolean same_caps = gst_caps_is_strictly_equal (caps, current_caps);

Put all the variable declarations at the beginning of the block, also a simple gst_caps_is_equal() is enough here :)
Comment 3 Joshua M. Doe 2014-09-23 20:50:06 UTC
Created attachment 286933 [details] [review]
moved decls up and did this for audiosrc as well

Implemented your suggestions, though my app doesn't use audio so that hasn't been tested yet.
Comment 4 Sebastian Dröge (slomo) 2014-09-24 07:03:00 UTC
commit 27eb6555d19ab2496cc35273bc54e89dfc0b9977
Author: Joshua M. Doe <oss@nvl.army.mil>
Date:   Thu Sep 18 16:13:58 2014 -0400

    dshowsrcwrapper: avoid error when set_caps called twice
    
    If set_caps is called in a running state, return immediately if the caps
    haven't changed. If the pins are already connected, disconnect them.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=736926