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 784831 - Seeking a pipeline containing concat only seeks the latest source
Seeking a pipeline containing concat only seeks the latest source
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
1.10.5
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-07-12 07:19 UTC by Jay Yang
Modified: 2018-11-03 12:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Code to reproduce the bug (2.70 KB, text/x-csrc)
2017-07-12 07:19 UTC, Jay Yang
Details

Description Jay Yang 2017-07-12 07:19:41 UTC
Created attachment 355384 [details]
Code to reproduce the bug

It seems that concat simply forwards the seek event to the current sink pad, and so if we try to seek to the beginning of the pipeline it only seeks part of the way.

I've attached a file that demonstrates the issue. It needs a test.ogg file that runs for 2 minutes. The code runs a pipeline consisting of two copies of test.ogg concatenated with concat. It runs the pipeline for 3 minutes and then tries to seek to the start. I would expect then the code to run for a total of 7 minutes, instead it runs for 5 minutes.

I'm on 1.10.5
Comment 1 Jan Schmidt 2017-10-29 15:46:11 UTC
As far as I know, this is the intended behaviour of the concat element. It doesn't logically concatenate all inputs streams to present one continuous timeline, it just handles gaplessly switching between sequential inputs.

If you're querying the playback duration, you'll notice that it changes when the inputs switch also.
Comment 2 Mathieu Duponchelle 2017-10-29 16:34:49 UTC
This corresponds to the currently intended behaviour. The behaviour you want would be a nice addition, but currently the implementation does not make it possible, as all pads except the currently-active one block immediately, which means concat can't know for example the total duration of its concatenated input streams.

To achieve this behaviour would involve accepting and queuing the mandatory start up events (stream-start / caps / segment) without blocking, which would let it present its output stream a bit more accurately (eg it could output a segment with an accurate duration, or know which pad to switch to when a seek event is sent).

Sorting this as enhancement, we'll be happy to give you advice on how to do this if you feel like tackling this :)
Comment 3 Nicolas Dufresne (ndufresne) 2017-10-29 19:59:22 UTC
Well no, to support duration and seeking the pad request would need to be frozen, while concat is designed so you can add more pads at run-time. I don't think it is worth keeping this bug for enhancement. I would make the seek ans duration query fail though, to avoid the unexpected.
Comment 4 Sebastian Dröge (slomo) 2017-10-30 07:46:17 UTC
(In reply to Nicolas Dufresne (stormer) from comment #3)
> I would make the seek ans duration query fail though, to avoid the unexpected.

You mean on the srcpad of concat? It intentionally works right now, proxying whatever information there is for the currently active pad.
Comment 5 Mathieu Duponchelle 2017-10-30 12:30:49 UTC
(In reply to Nicolas Dufresne (stormer) from comment #3)
> Well no, to support duration and seeking the pad request would need to be
> frozen

Why?
Comment 6 Nicolas Dufresne (ndufresne) 2017-10-30 14:58:32 UTC
(In reply to Mathieu Duponchelle from comment #5)
> (In reply to Nicolas Dufresne (stormer) from comment #3)
> > Well no, to support duration and seeking the pad request would need to be
> > frozen
> 
> Why?

Duration will change (not a big deal, but most app don't support that), it would also be racy with the READY to PAUSED transition (hopefully seek will fail, but it is also often unexpected by app).

(In reply to Sebastian Dröge (slomo) from comment #4)
> (In reply to Nicolas Dufresne (stormer) from comment #3)
> > I would make the seek ans duration query fail though, to avoid the unexpected.
> 
> You mean on the srcpad of concat? It intentionally works right now, proxying
> whatever information there is for the currently active pad.

Yes, though proxying will make it behave as if there was only 1 source. I sense a behaviour break if someone implement seeking over all sources. All in all, if you designed it this way, you should maybe document it (and maybe add tests if not done already).
Comment 7 Mathieu Duponchelle 2017-10-30 19:50:04 UTC
> Duration will change (not a big deal, but most app don't support that)

Yeah, that would kind of be the point of such a feature :)

> it would also be racy with the READY to PAUSED transition (hopefully seek will
> fail, but it is also often unexpected by app).

What exactly would be racy?
Comment 8 Nicolas Dufresne (ndufresne) 2017-10-30 23:53:19 UTC
If a seek occurs before the source has reached paused state, it's likely that the seek will fail. It is then a race between the application call and the source thread processing the state change.
Comment 9 Mathieu Duponchelle 2017-10-31 13:44:42 UTC
(In reply to Nicolas Dufresne (stormer) from comment #8)
> If a seek occurs before the source has reached paused state, it's likely
> that the seek will fail. It is then a race between the application call and
> the source thread processing the state change.

Well, seeking on ready isn't something we really support everywhere anyway is it? I'm not sure how this is related to this specific issue to be honest :)
Comment 10 Nicolas Dufresne (ndufresne) 2017-10-31 14:28:15 UTC
Seek on ready would fix the describe race, I'm sorry that I don't have other words to explain the details. Best at this point would be for you to go and implement it, and then wrap this as some source with URI and run validate tests on top. Then you should be able to report the real life found issues. You also need to find a solution to the fact the current behaviour is intentional but not documented, as reported by Sebastien in comment 4.

I don't believe the concat design will hold, it's not like splitmuxsrc, which was design with all these details in mind. But I'll be happy if someone proof otherwise.
Comment 11 GStreamer system administrator 2018-11-03 12:42:03 UTC
-- 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/gstreamer/issues/245.