GNOME Bugzilla – Bug 744783
gst-validate: support for non flushing seeks
Last modified: 2018-11-03 11:06:32 UTC
Placeholder description so I get a bug number to add in the commit message.
Created attachment 297263 [details] [review] support for non flushing seeks
Review of attachment 297263 [details] [review]: Please add validate: in the commit message I wonder if it would not make more sense to wait for a new segment corresponding to the seek in that case?
Yes, it'd seem better, I'll change the patch to do that.
Created attachment 297637 [details] [review] support for non flushing seeks
For info, https://bugzilla.gnome.org/show_bug.cgi?id=729198 is the patch to oggdemux that led to this.
Review of attachment 297637 [details] [review]: ::: validate/gst/validate/gst-validate-scenario.c @@ +415,3 @@ +static GstObject * +_find_any_object (GstIterator * iter) +{ I think we should wait for the new old segment to be done on all sinks to be able to trigger the next action.
Review of attachment 297637 [details] [review]: ::: validate/launcher/apps/gstvalidate.py @@ +719,3 @@ "fast_forward", "seek_forward", + "seek_forward_non_flushing", Adding it this way will lead to many new tests added in the validate testsuite, did you run them all?
From your wording, do you mean automatic ? I did not know these would be automatically enrolled in anything, I first added new tests scenarios, and grepped around when I realized they were not seen, and added them to the list where other scenarios were found. I tested with: ./tools/gst-validate-1.0 souphttpsrc location=http://127.0.0.1:8000/test.ogg ! oggdemux ! vorbisdec ! pulsesink volume=0.01 --set-scenario seek_forward_non_flushing Can you expand a bit on what's automatic and from where in the code ?
(In reply to Vincent Penquerc'h from comment #8) > From your wording, do you mean automatic ? > I did not know these would be automatically enrolled in anything, I first > added new tests scenarios, and grepped around when I realized they were not > seen, and added them to the list where other scenarios were found. > > I tested with: > > ./tools/gst-validate-1.0 souphttpsrc location=http://127.0.0.1:8000/test.ogg > ! oggdemux ! vorbisdec ! pulsesink volume=0.01 --set-scenario > seek_forward_non_flushing > > Can you expand a bit on what's automatic and from where in the code ? Yes, it will automatically add more tests to the default testsuite: http://cgit.freedesktop.org/gstreamer/gst-integration-testsuites/tree/testsuites/validate.py You should run gst-validate-launcher validate (--mute) to see how it works with other formats.
Created attachment 297785 [details] [review] support for non flushing seeks Attached only for the change to wait for a segment on all sinks. I have yet to look at the automated use of new scenarii.
Created attachment 297892 [details] [review] support for non flushing seeks
Created attachment 297893 [details] [review] add non lushing seek scenarios
Review of attachment 297892 [details] [review]: ::: validate/gst/validate/gst-validate-scenario.c @@ +600,3 @@ + /* Flushing seeks will be deemed done when an ASYNC_DONE message gets + received on the bus. We don't get one for non flushing seeks though, + so we rely on a discontinuity on the buffers. */ We do not rely on discontunuity anymore :) I wonder if we could not alway rely on segments actually, that might even cleaner as we check the seqnum which guarantees us the segment is the resultant of the seek.
Created attachment 297894 [details] [review] support for non flushing seeks Comment fixed to mention segments instead of disconts :)
Review of attachment 297894 [details] [review]: OK
Review of attachment 297894 [details] [review]: ::: validate/data/seek_backward_non_flushing.scenario @@ +1,1 @@ +description, seek=true, duration=30, need-clock-sync=true Actually please add min-media-duration=15 here. ::: validate/data/seek_forward_non_flushing.scenario @@ +1,1 @@ +description, seek=true, duration=20, need-clock-sync=true Please add min-media-duration=30 here.
Created attachment 297903 [details] [review] add non flushing seek support
Created attachment 298091 [details] [review] use segments for flushing seek too
Review of attachment 298091 [details] [review]: This means we will consider we can seek again even though we're not ASYNC_DONE, as ASYNC_DONE is emitted on the first buffer, not sure it's really right?
(In reply to Mathieu Duponchelle from comment #19) > Review of attachment 298091 [details] [review] [review]: > > This means we will consider we can seek again even though we're not > ASYNC_DONE, as ASYNC_DONE is emitted on the first buffer, not sure it's > really right? I have the impression you are right, we should wait for the segment with right seqnum + 1 buffer, and that even on segment seeks I think.
That's a good point, I'll go change that.
We could also wait for segment + ASYNC_DONE methinks
Created attachment 298105 [details] [review] expect a buffer with discontinuity after a seek
Review of attachment 298105 [details] [review]: Looks good to me. Have you tried these patches with a full run of gst-validate-launcher ?
I tested with: gst-validate-launcher validate -j1 --mute The failures are unrelated.
OK to push ?
I think we are all fine yes
I appear to not have permissions to push, so I'll have to leave that to you :) I just merged with latest master and it all still builds fine.
Why don't you have permissions? You can push on Fdo no?
I can push to core, -base, etc. Trying to push to gst-devtools gives me: fatal: remote error: access denied or repository not exported: /gstreamer/gst-devtools If this repo is supposed to have the same permissions to core or -base, then it might be something local.
Oh, nevermind, I see the problem. It was trying to push to anongit :D Pushed.
Those patches were reverted: commit 3bb6ecd6fabb7f84348cfd62990655d2292b64ea Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Mon Mar 9 18:26:37 2015 +0000 Revert "validate: add non flushing seek support" This reverts commit 3ff55dcc3119b39e7c86044159db8bce49a2dc3a. Regressions on the test server, apparently linked to this patchset. commit 09988a97bf136b54f71bcbcbbae45459b0d5d69c Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Mon Mar 9 18:26:33 2015 +0000 Revert "validate: use segments to detect success of flushing seeks too" This reverts commit c47cc7ba90e96ffaefe201087428ef448670f3be. Regressions on the test server, apparently linked to this patchset. commit d78c20322f287a0d84043df5860d094a3767cce8 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Mon Mar 9 18:26:06 2015 +0000 Revert "validate: expect a buffer with discontinuity after a seek" This reverts commit 87064b6994e36203b6976d436feda809068f1497. Regressions on the test server, apparently linked to this patchset.
Trying those patches again, most tests succeed. I see three tests failing that use those new non-flushing scenarios: First one: GST_GL_XINITTHREADS=1 GST_VALIDATE_OVERRIDE=:/home/v/gst-validate/gst-integration-testsuites/medias/defaults/online-streams-infos/dash/dash.exMPD_BIP_TC1.override DISPLAY=:0.0 GST_VALIDATE_SCENARIO=seek_backward_non_flushing gst-validate-1.0 playbin uri=http://127.0.0.1:8079/defaults/exMPD_BIP_TC1/exMPD_BIP_TC1.mpd audio-sink='fakesink sync=true' video-sink='fakesink sync=true' --set-media-info "/home/v/gst-validate/gst-integration-testsuites/medias/defaults/online-streams-infos/dash/dash.exMPD_BIP_TC1.stream_info" Failed 'Application returned 18 (issues: [We got an ERROR message on the bus]) The error on the bus is a 404. The source for this seems to not be on my disk (I an --sync less than two hours ago): v@euryale:~/src/gst-integration-testsuites/medias$ ls -l defaults/exMPD_BIP_TC1/exMPD_BIP_TC1.mpd lrwxrwxrwx 1 v v 201 Aug 21 13:37 defaults/exMPD_BIP_TC1/exMPD_BIP_TC1.mpd -> ../../../.git/annex/objects/93/wW/SHA256E-s1489--32f5584b49364d27e4a8c75bb276412e37a09eb6e81413d146c32c92a089578c.mpd/SHA256E-s1489--32f5584b49364d27e4a8c75bb276412e37a09eb6e81413d146c32c92a089578c.mpd v@euryale:~/src/gst-integration-testsuites/medias$ readlink -f defaults/exMPD_BIP_TC1/exMPD_BIP_TC1.mpd Second one: Same as first one, with the seek_forward_non_flushing. Third one: GST_GL_XINITTHREADS=1 DISPLAY=:0.0 GST_VALIDATE_SCENARIO=seek_backward_non_flushing gst-validate-1.0 playbin uri=file:///home/v/gst-validate/gst-integration-testsuites/medias/defaults/mpegts/tron_en_ge_aac_h264.ts audio-sink='fakesink sync=true' video-sink='fakesink sync=true' --set-media-info "/home/v/gst-validate/gst-integration-testsuites/medias/defaults/mpegts/tron_en_ge_aac_h264.ts.media_info" Three subsequent manual rerun worked.
Gonna close this again. Not useful to keep a "lots of stuff fails" bug open. Let's file a new issues about specific things if anything still fails 3 years later.
I have a better implementation for this, assigning to myself.
I have pushed a series of patches that should open the way to handling all types of seeks in a reliable fashion: https://cgit.freedesktop.org/~bilboed/gst-devtools/log/?h=flushless
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-devtools/issues/11.