GNOME Bugzilla – Bug 757776
dashdemux: Unit test fails reliably
Last modified: 2015-11-24 17:13:12 UTC
See for example: https://ci.gstreamer.net/job/GStreamer-master/5587/testReport/
The problems started with commit ccff3be3ab2e5bffcefc12c80a5edb225801f1b9. That commit replaced the HTTPsrc element that adaptivedemux was using with a bin element and made the input generation asynchronous. The tests relied on 2 facts: 1) the input is a fakeHTTPsrc element 2) the input generation and adaptivedemux output processing is done in the same thread. To fix the problems we need to: 1) make fakeHTTPsrc thread safe (the configuration functions can now be called from another thread) 2) update tests to correctly reach the fakeHTTPsrc element in the bin so that they can configure the generation of a download error 3) update tests to block the input generation on certain events (so that we can simulate a download error before the asynchronous input generation finishes pushing all data to the queue) 1 and 2 are easy. 3 might be tricky
Florin, are you working on this or should I take it?
Created attachment 315159 [details] [review] patch 1/3
Created attachment 315160 [details] [review] patch 2/3
Created attachment 315161 [details] [review] patch 3/3
Fixed major problems, but there are 2 open issues: 1. we need access to create function from fakeHTTPsrc element. Me and Alex Ashley are working on refactoring the test engine, so testFragmentDownloadError is disabled until that is available. 2. There are some memory leaks in all tests. I've traced the leaks to the newly added bin in adaptive demux, but I cannot figure out who is holding references to this new bin. I will raise a new ticket for that. With above patches, the test are passing.
https://bugzilla.gnome.org/show_bug.cgi?id=757859 is tracking the memory leaks
Thank you, Florin!
Review of attachment 315160 [details] [review]: ::: tests/check/elements/dash_demux.c @@ +1110,3 @@ + /* fakeHTTPsrc element is placed inside a bin. Get the bin */ + srcBin = gst_object_get_parent (msg->src); I think this object leaks
Nevermind, I just fixed it, it's trivial. I'll push them all once built/tested.
commit c6243c7d0085ee3a40f58dfb9d72a1fede6dda21 Author: Florin Apostol <florin.apostol@oregan.net> Date: Mon Nov 9 18:07:30 2015 +0000 adaptivedemux: tests: disabled testFragmentDownloadError test Until we will have support to control the generating thread from fakeHTTPsrc element, the test testFragmentDownloadError is disabled. https://bugzilla.gnome.org/show_bug.cgi?id=757776 commit 665eb6fccae793bd1ed5437b28e0eb8c0aec15aa Author: Florin Apostol <florin.apostol@oregan.net> Date: Mon Nov 9 14:14:34 2015 +0000 adaptivedemux: tests: corrected access to fakeHTTPsrc element The src element for adaptivedemux is now a bin. Updated the tests to correctly reach into the bin and get the fakeHTTPsrc element https://bugzilla.gnome.org/show_bug.cgi?id=757776 commit 7f3d47deb3883b3b0d52a6cc739fd5b373767a3b Author: Florin Apostol <florin.apostol@oregan.net> Date: Mon Nov 9 14:13:04 2015 +0000 adaptivedemux: tests: made fakeHTTPsrc element MT safe https://bugzilla.gnome.org/show_bug.cgi?id=757776
(In reply to Vincent Penquerc'h from comment #9) > Review of attachment 315160 [details] [review] [review]: > > ::: tests/check/elements/dash_demux.c > @@ +1110,3 @@ > > + /* fakeHTTPsrc element is placed inside a bin. Get the bin */ > + srcBin = gst_object_get_parent (msg->src); > > I think this object leaks Thanks for spotting it. I don't know why valgrind didn't pick this.