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 768723 - rtprtx: test is sometimes failing
rtprtx: test is sometimes failing
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-07-12 11:26 UTC by Guillaume Desmottes
Modified: 2016-11-27 11:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rtprtxsend: Update statistics before pushing (1.38 KB, patch)
2016-11-27 10:15 UTC, Edward Hervey
committed Details | Review

Description Guillaume Desmottes 2016-07-12 11:26:41 UTC
The 'elements/rtprtx' test is sometimes failing here with master.

elements/rtprtx.c:163:F:general:test_push_forward_seq:0: 'nbrtxpackets' (2) is not equal to '3' (3)
Comment 1 Tim-Philipp Müller 2016-07-12 11:42:37 UTC
I could swear there's a bug for this already, but can't find it right now.

Has been failing for eternities, possibly since rtx support was added.
Comment 2 Guillaume Desmottes 2016-07-12 13:33:59 UTC
I also observed this one:

gstcheck.c:391:F:general:test_rtxreceive_data_reconstruction:0: check pad_peer (0x18324e0) refcount is 3 instead of 2

Here is the full logs as well: http://people.collabora.co.uk/~cassidy/gst-bgo768723.log.gz
Comment 3 Edward Hervey 2016-11-26 12:11:59 UTC
There is also this one : test_rtxsend_rtxreceive:0: 'rtx_packets' (4) is not equal to 'packets_num' (5)

After digging further, the issue is a bit ... stupid.

rtx_packets is a counter which is incremented in rtprtxsend *after* the buffer was pushed.

So sometimes when you query the value ... it hasn't incremented it yet (just after the push).

Is there a way we can guarantee with harness that the push has completely come back ?
Comment 4 Sebastian Dröge (slomo) 2016-11-27 09:51:04 UTC
Maybe Håvard has an idea?
Comment 5 Håvard Graff (hgr) 2016-11-27 09:55:08 UTC
(In reply to Sebastian Dröge (slomo) from comment #4)
> Maybe Håvard has an idea?

If you pull out all buffers (with gst_harness_pull()) you get this guarantee. Maybe the test does not completely pull out all available ones? (a classic source of race-conditions!)
Comment 6 Edward Hervey 2016-11-27 10:15:54 UTC
Created attachment 340836 [details] [review]
rtprtxsend: Update statistics before pushing

If an element queries the number of retransmission buffers pushed
*while* the push is still taking place (and before the object lock
is taken just after) it would end up with the wrong statistic
being reported.

Increment it just before the push, avoids races when getting statistics
Comment 7 Edward Hervey 2016-11-27 11:23:23 UTC
commit 91f5b4eaa2fcd874361d16bdf80497c3cdd12d13
Author: Edward Hervey <edward@centricular.com>
Date:   Sun Nov 27 11:14:13 2016 +0100

    rtprtxsend: Update statistics before pushing
    
    If an element queries the number of retransmission buffers pushed
    *while* the push is still taking place (and before the object lock
    is taken just after) it would end up with the wrong statistic
    being reported.
    
    Increment it just before the push, avoids races when getting statistics
    
    https://bugzilla.gnome.org/show_bug.cgi?id=768723