GNOME Bugzilla – Bug 794149
basesrc: regression with giosrc ssh:// protocol
Last modified: 2018-03-08 13:46:15 UTC
$ totem ssh://server/path/to/file $ gst-play-1.0 ssh://server/path/to/file Don't seem to work any more in git master, whereas the same works with 1.12.4.
This is caused by commit 523de1a9dc7b7f79c78120bed15c364336f067cb Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Thu Jun 1 10:36:26 2017 -0400 basesrc: Don't hold LIVE_LOCK in create/alloc/fill Holding this lock on live source prevents the source from changing the caps in ::create() without risking a deadlock. This has consequences as the LIVE_LOCK was replacing the STREAM_LOCK in many situation. As a side effect: - We no longer need to unlock when doing play/pause as the LIVE_LOCK isn't held. We then let the create() call finish, but will block if the state have changed meanwhile. This has the benefit that wait_preroll() calls in subclass is no longer needed. - We no longer need to change the state to unlock, simplifying the set_flushing() interface - We need different handling for EOS depending if we are in push or pull mode. This patch also document the locking of each private class member and the locking order. https://bugzilla.gnome.org/show_bug.cgi?id=783301 What happens is that unlock() is called, and then afterwards start() is called without unlock_stop() first. So the cancellable is still cancelled, and all the GIO operations in start() just error out because of that.
Created attachment 369404 [details] log Relevant part of the log. Line 4 and 5 seem to be in the wrong order (first unlock_stop is called, then unlock).
Created attachment 369410 [details] [review] basesrc: No need to stop flushing in start_complete The flushing state is handled a bit differently, there is no need to stop flushing in start_complete. This would other result in unlock_stop being called without unlock_start. Unlike what the old comment says, there is no need to take the live lock here, we are still single threaded at this point (app thread or the state change thread). Also, we will wait for playing state in create/getrange, no need to do that twice.
Created attachment 369411 [details] [review] basesrc: Balance unlock/unlock_stop in _src_stop() Otherwise it's possible that we won't be able to start again depending the implementation. We do start/stop in normal use cases whenever GST_QUERY_SCHEDULING happens before we are started.
This passes the unit tests and fixes the regression we see here.
Just ran validate locally and it's clean to.
Attachment 369410 [details] pushed as 12c5d90 - basesrc: No need to stop flushing in start_complete Attachment 369411 [details] pushed as cbd03e2 - basesrc: Balance unlock/unlock_stop in _src_stop()