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 588744 - [check] Stream consistency checker utility
[check] Stream consistency checker utility
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 588748
 
 
Reported: 2009-07-16 07:02 UTC by Edward Hervey
Modified: 2009-07-28 15:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gstcheck: Add a stream consistency checking helper routine. (8.73 KB, patch)
2009-07-16 07:03 UTC, Edward Hervey
committed Details | Review
check: make new GstStreamConsistency structure private (2.73 KB, patch)
2009-07-28 14:28 UTC, Tim-Philipp Müller
committed Details | Review

Description Edward Hervey 2009-07-16 07:02:52 UTC
Many elements don't have any checks (whether internally or in the unit tests) to make sure that the outgoing data flow is consistant.

* No in-band data (buffers or events) are sent before NEWSEGMENT
* NEWSEGMENT is sent out after a FLUSH_START/FLUSH_STOP tuple
* Only *ONE* FLUSH_STOP follows a FLUSH_START
* No in-band data (buffers or events) are sent after EOS
* No in-band data (buffers or events) are sent between FLUSH_START and FLUSH_STOP
* ... maybe some more

For this I've created a StreamConsistency helper utility for libgstcheck which can be used in various unit tests. This has already helped detect a few bugs in some elements.

Doing this is very important for dynamic pipelines (in general and GNonLin ones in particular) where a certain data flow consistency is expected.
Comment 1 Edward Hervey 2009-07-16 07:03:26 UTC
Created attachment 138506 [details] [review]
gstcheck: Add a stream consistency checking helper routine.
Comment 2 Edward Hervey 2009-07-20 06:44:39 UTC
ouch, I meant to mark this one as a blocker too :(
Comment 3 Jan Schmidt 2009-07-20 08:02:18 UTC
Looks ok. The check about receiving a new-segment between flush-start and flush-stop sounds dubious to me though: While forwarding flush-start from one thread, an element will continue as it was in the streaming thread until it gets a WRONG_STATE return from pushing something and can return it upstream. That means it might indeed push a new-segment during the flush.
Comment 4 Edward Hervey 2009-07-20 08:45:26 UTC
Good point, I thought the GST_PAD_IS_FLUSHING check was done on source pads... but it appears it's only done on the peerpad.

I'll remove the flushing checks, those checks are already handled by code in core (gstpad).
Comment 5 Edward Hervey 2009-07-20 08:46:44 UTC
commit dcdc73d1826d3dca91663ee8ce8af84fcf3ea317
Author: Edward Hervey <bilboed@bilboed.com>
Date:   Mon Jul 13 09:22:06 2009 +0200

    gstcheck: Add a stream consistency checking helper routine. Fixes #588744

Comment 6 Tim-Philipp Müller 2009-07-27 22:27:58 UTC
Sorry for re-opening. Two quick questions:

 - why is the GstStreamConsistency structure public? Does it
   need to be public?

 - if it should be public, shouldn't it have padding?
Comment 7 Edward Hervey 2009-07-28 11:28:24 UTC
good point... it doesn't need to be public (at least not the contents).
Comment 8 Tim-Philipp Müller 2009-07-28 14:28:34 UTC
Created attachment 139384 [details] [review]
check: make new GstStreamConsistency structure private
Comment 9 Jan Schmidt 2009-07-28 15:28:40 UTC
Patch looks fine to me. If bilboed is OK with it too, push it...
Comment 10 Tim-Philipp Müller 2009-07-28 15:43:13 UTC
commit 623c19983a6e6e099d53b681a1ece5883d2f5fac
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Tue Jul 28 15:23:15 2009 +0100

    check: make new GstStreamConsistency structure private
    
    There's no need to have GstStreamConsistency in a public header for
    the time being, so make it private. While we're at it, add a gtk-doc
    blurb for it though. Re-fixes #588744.