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 667716 - decklinksrc: doesn't push new segment event on all pads
decklinksrc: doesn't push new segment event on all pads
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
0.10.22
Other Linux
: Normal normal
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-11 21:18 UTC by blake tregre
Modified: 2012-03-17 21:29 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
patch that pushes event on both pads (1.45 KB, patch)
2012-01-11 21:18 UTC, blake tregre 		none Details | Review
updated patch (1.55 KB, patch)
2012-01-18 19:07 UTC, blake tregre 		none Details | Review
patch favoring video (1.67 KB, patch)
2012-02-12 06:54 UTC, blake tregre 		none Details | Review

Description blake tregre 2012-01-11 21:18:03 UTC 
Created attachment 205046 [details] [review]
patch that pushes event on both pads

Currently, decklinksrc only pushes new segment events on the videosrc pad.  I think that it should push the event on both pads, so I've attached a little patch to do that.
Comment 1 Vincent Penquerc'h 2012-01-18 11:05:23 UTC 
I believe the extra event ref should be within the pad linked check to avoid leaks (and manually unreffed if both link checks return FALSE, for safety).
Comment 2 blake tregre 2012-01-18 19:07:12 UTC 
Created attachment 205561 [details] [review]
updated patch

Ah, you're right.  I've made sure to unref the event if not linked.  Thanks.
Comment 3 Vincent Penquerc'h 2012-01-23 11:54:53 UTC 
Failure to push the event on the first pad will still leak the event here, due to the return statement.
This would be fixed if the extra event ref should be within the pad linked check.

To be honest I'm not sure if an error pushing the event should or should not still push to the other pad...
Comment 4 blake tregre 2012-02-12 06:54:51 UTC 
Created attachment 207380 [details] [review]
patch favoring video

Sorry to let the conversation lapse.  I should have noticed the potential for leakage, but I was attracted by the symmetry of the solution.

I'm not really sure what should happen there either.  I think the code expresses a preference for video over audio buffers, so perhaps we should be more concerned with the ability to push the event over the video src pad.  With that in mind, I've a new patch.
Comment 5 David Schleef 2012-03-17 21:29:48 UTC 
commit 86eeca91de0cd5143ef23355cd8d4792d13e7b71
Author: blake tregre <blake@oblong.com>
Date:   Sat Feb 11 22:49:10 2012 -0800

    decklinksrc: push new new segment event to all pads
    
    Take care to push the event to all pads, but favor the video src pad.
    Fixes: #667716.