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 708165 - videomixer: Store and forward tag events
videomixer: Store and forward tag events
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-09-16 15:02 UTC by Mathieu Duponchelle
Modified: 2013-11-29 19:56 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
fixes the reported issue (2.82 KB, patch)
2013-09-16 15:02 UTC, Mathieu Duponchelle
needs-work Details | Review
New patch to correct the issue (3.15 KB, patch)
2013-10-28 14:08 UTC, Mathieu Duponchelle
none Details | Review

Description Mathieu Duponchelle 2013-09-16 15:02:58 UTC
Created attachment 255035 [details] [review]
fixes the reported issue

We have race conditions highlighted by our integration test suite where these events were stickied and pushed on a flushing pad, send_event would just return FALSE (which, as already discussed in other instances, might be the real bug but it seemed it was too much of a hassle to change the prototype)

The attached patch mimics adder's behaviour, which is to save these tags and forward them in collected.

Should we open another bug "change send_event prototype eventually" ?
Comment 1 Olivier Crête 2013-09-23 19:57:38 UTC
Looks good, but why only tag events? Why not also all other sticky events ?
Comment 2 Sebastian Dröge (slomo) 2013-09-24 08:22:06 UTC
Comment on attachment 255035 [details] [review]
fixes the reported issue

Not sure how to best handle other events too here.

However this is not correct for tags. You need to accumulate/merge tags and always send the complete, currently valid tags downstream. I guess this also needs changes in adder then.
Comment 3 Mathieu Duponchelle 2013-10-28 14:08:54 UTC
Created attachment 258306 [details] [review]
New patch to correct the issue
Comment 4 Mathieu Duponchelle 2013-10-28 14:10:19 UTC
Not sure about the merge macro to use, MERGE_APPEND seemed good to me but it might not be the good one.
Comment 5 Thibault Saunier 2013-11-22 22:03:43 UTC
commit 83f8ee1d41a238b2fc54c127eee8dee33390d08f
Author: MathieuDuponchelle <mathieu.duponchelle@epitech.eu>
Date:   Sat Sep 14 03:27:09 2013 +0200

    videomixer2: Merge tag events to send them in collected.
    
    Otherwise there were race conditions where we would send tags
    on a flushing srcpad.
    
    We have a test for that in GES, but this should be tested
    systematically with harness in the future as I believe it
    is useful for exactly that kind of cases.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=708165