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 787405 - [REGRESSION] - Seeking in NLE is broken
[REGRESSION] - Seeking in NLE is broken
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-editing-services
1.x
Other Linux
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-09-07 11:39 UTC by Thibault Saunier
Modified: 2017-09-19 15:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
WIP Always execute seeks (1.22 KB, patch)
2017-09-07 17:59 UTC, Nicolas Dufresne (ndufresne)
none Details | Review
WIP: nlecomposition: Don't wait for a buffer to start the task (1.04 KB, patch)
2017-09-15 19:44 UTC, Nicolas Dufresne (ndufresne)
none Details | Review
nlecomposition: Also start task on allocation query (1.74 KB, patch)
2017-09-15 21:32 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
nlecomposition: Always execute seeks (1.76 KB, patch)
2017-09-15 21:45 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Thibault Saunier 2017-09-07 11:39:34 UTC
Since https://ci.gstreamer.net/job/Gstreamer-Manifest-Update/9434/ (ie. adding support for allocation query in tee), the GES seeking integration tests fail:

https://ci.gstreamer.net/job/GStreamer-master-validate/lastCompletedBuild/testReport/

The tests time out (no deadlock but all thread are waiting).
Comment 1 Nicolas Dufresne (ndufresne) 2017-09-07 16:49:29 UTC
So far, what I see:
  - nlecomposition queue an incoming seek
  - The pipeline is in paused, blocked on wait-for-preroll
  - Nothing happens

Normally, when seeks aren't queued, a flush operation will unblock the preroll, and everything should be fine. Dropping the allocation query (hence not blocking in while executing gst_aggregator_do_allocation() seemed to work, but was of course just hiding an existing bug.
Comment 2 Nicolas Dufresne (ndufresne) 2017-09-07 16:52:55 UTC
About the task that is supposed to run the seek, it's not running anymore, according to what I read, it's suppose to run always while in READY + state.
Comment 3 Nicolas Dufresne (ndufresne) 2017-09-07 17:59:23 UTC
Created attachment 359370 [details] [review]
WIP Always execute seeks

Ignoring the seeks results in stalled pipeline when tee forward the
allocation queries.

It's possible that all we want is to execute the flush sequence, though don't
ask me why this "optimization" was put in place, I can't figure-out. In general,
when I run a flushing seek from my application, I expect a flush and a seek to
happen.
Comment 4 Nicolas Dufresne (ndufresne) 2017-09-15 19:44:08 UTC
Created attachment 359873 [details] [review]
WIP: nlecomposition: Don't wait for a buffer to start the task

If the pipeline is paused, the allocation query may block, hence we may
never get a buffer.
Comment 5 Nicolas Dufresne (ndufresne) 2017-09-15 20:01:58 UTC
Just piling hacks as I'm building knowledge around what this code is doing. So now I think I understand the idea with dropping seeks (which I've disabled in first patch). I think it is to avoid a userless seek that would for sure return EOS. Something like:

  seek, EOS, change stack, implicit-seek

I think it's fine to drop the first seek, but  not the associated flush, which won't be forwarded downstream when the "implicit-seek" happens.

For the second issue, which gst-validate didn't pick up, but I was seeking in PiTiVi, it's really simple. The audio sink is in paused, there is buffer, caps segment in the queue already and it's blocked on allocation query. The seek is queued, but the task that will exectute this seek is waiting for a buffer (which is the current condition for a stack to be ready). So if I simply assume stack is ready on segment, this situation is avoided. events don't take any "space" in queues, so they should not block on downstream queue. These two event are definitely expected before the allocation query take place.
Comment 6 Nicolas Dufresne (ndufresne) 2017-09-15 21:32:06 UTC
Created attachment 359877 [details] [review]
nlecomposition: Also start task on allocation query

The allocation query may block on the sink when in pause. As a side effect, we
may never get a buffer now that tee does forward the allocation query.
This would often lead in a pipeline stall.
Comment 7 Nicolas Dufresne (ndufresne) 2017-09-15 21:36:39 UTC
Comment on attachment 359370 [details] [review]
WIP Always execute seeks

Actually this one is still needed according to gst-validate. I think we need to minimally run the FLUSH, but that's much more complicated then just letting the seek run on the dead stack.
Comment 8 Nicolas Dufresne (ndufresne) 2017-09-15 21:45:09 UTC
Created attachment 359878 [details] [review]
nlecomposition: Always execute seeks

We have an optiominisation to avoid double seeks when a seek is passed
the end of the current stack. The problem, is that we no longer flush
the pipeline when this code is reached. This patch comments out this
optimization adding a FIXME. As mention, flushing the stack instead of
seeking would work, but does not seem trivial considering all the
mechanic inplace to forward or not the events.
Comment 9 Thibault Saunier 2017-09-19 15:41:15 UTC
I made some minor fixes.,

Thanks for investigating!

Attachment 359877 [details] pushed as faf44ac - nlecomposition: Also start task on allocation query
Attachment 359878 [details] pushed as 5dd5b16 - nlecomposition: Always execute seeks