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 708182 - Refactor / rearchitect the pipeline and merge the seeker class
Refactor / rearchitect the pipeline and merge the seeker class
Status: RESOLVED OBSOLETE
Product: pitivi
Classification: Other
Component: Playback
0.91
Other Linux
: Normal normal
: 0.95
Assigned To: Thibault Saunier
Pitivi maintainers
: 707189 (view as bug list)
Depends on:
Blocks: 708548
 
 
Reported: 2013-09-16 18:35 UTC by Jean-François Fortin Tam
Modified: 2015-10-20 13:08 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Jean-François Fortin Tam 2013-09-16 18:35:00 UTC
utils/pipeline.py has multiple classes that kind of overlap with each other. Among other things, this leads to situations where we're not sure when we must use Seeker or Pipeline or SimplePipeline to do things like async/idle seeks etc. There are quite a lot of errors that don't get handled, we sometimes spam the lower levels with seek requests (ex: when scrubbing with the ruler), etc.

------------
<nekohayo> thiblahute, got a little surprise for you... I coerced luisbg into taking a quick look at how we spam the seeker when scrubbing... two commits were born: https://github.com/nekohayo/pitivi/commits/seeking-fixes
 there is a remaining issue that you could fix though: the Seeker class uses this 80 miliseconds time-based timeout, shouldn't it wait for async_done like what you just did in the pipeline class in commit b5f70a7535e? if our suspicion is correct, you could do that and make it more robust that way

<luisbg> current code works, but waiting based on time isn't ideal, waiting for gstreamer to finish the previous seek is better
<thiblahute> luisbg, We do wait for Gst to finish afterwards
<thiblahute> And seek are already compressed.
<nekohayo> in Seeker?
<thiblahute> In the pipeline
<thiblahute> We should remove the seeker, but I did not want to do it now as it is too invasive

<luisbg> nekohayo, it is what you were saying at the start... why 3 similar classes instead of merging all code to one
<thiblahute> luisbg, https://git.gnome.org/browse/pitivi/commit/?id=b5f70a7535e0a7
<luisbg> thiblahute, yes :) nekohayo showed me
<luisbg> but it is in SimplePipeline, and not in Seeker, which is why we mentioned it would be cool to have it in both... but maybe not worth it if dropping Seeker is a plan
<thiblahute> Right :)
Comment 1 Jean-François Fortin Tam 2013-09-19 21:09:07 UTC
From bug 707189:

"Currently we have a timeout based seeker and this is ugly/not optimal/not the
proper way. We can have the information from Gst about when the next seek can be done, and this is through the ASYNC_DONE bus message.

All the Seeker code logic should probably be moved to Pipeline, and we should not use any timeout, and instead wait for that message on the bus."


(note: the ASYNC_DONE stuff was done in the Pipeline class now, just not in the rest such as the seeker class)
Comment 2 Jean-François Fortin Tam 2013-09-19 21:09:29 UTC
*** Bug 707189 has been marked as a duplicate of this bug. ***
Comment 3 Jean-François Fortin Tam 2015-03-16 19:39:43 UTC
Thibault, is this considered done for 0.95?
Comment 4 Thibault Saunier 2015-10-20 13:08:31 UTC
This bug has been migrated to https://phabricator.freedesktop.org/T3055.

Please use the Phabricator interface to report further bugs by creating a task and associating it with Project: Pitivi.

See http://wiki.pitivi.org/wiki/Bug_reporting for details.