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 750013 - playbin: seeking halts playback with gst-play-1.0 gapless
playbin: seeking halts playback with gst-play-1.0 gapless
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.4.5
Other Linux
: Normal normal
: 1.5.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 750437 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-05-28 08:28 UTC by matsumura
Modified: 2016-01-10 19:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
log file of case3 (40.39 KB, text/plain)
2015-05-28 08:28 UTC, matsumura
  Details
the patch to fix the invalid base time (557 bytes, patch)
2015-06-08 00:29 UTC, matsumura
rejected Details | Review
test7 added seek command (6.03 KB, text/x-csrc)
2015-06-17 01:13 UTC, matsumura
  Details
streamsynchronizer: Ignore flushing streams [..] (1.47 KB, patch)
2016-01-09 03:44 UTC, Mathieu Duponchelle
rejected Details | Review

Description matsumura 2015-05-28 08:28:57 UTC
Created attachment 304131 [details]
log file of case3

I tried gapless playback of 3 flac songs following command.

gst-play-1.0 --gapless --interactive file:///1.flac file:///2.flac file:///3.flac

Is it bug?

-case1 OK
problem doesn't occur unless seeking.

-case2 OK
When the following operation is performed, a problem doesn't occur.
a.started playing 1.flac.
b.seek to forward push ST_PLAY_KB_ARROW_RIGHT key several times.
c.about-to-finish
d.started playing 2.flac.
e.seek to forward push ST_PLAY_KB_ARROW_RIGHT key several times.
f.about-to-finish
g.started playing 3.flac.

-case3 NG
When the following operation is performed, a problem occurs.
a.started playing 1.flac.
b.wait
c.about-to-finish
d.started playing 2.flac.
e.seek to forward push ST_PLAY_KB_ARROW_RIGHT key several times.
f.about-to-finish
g.started playing 3.flac.
h.playback halted.

see attached log file.
Comment 1 matsumura 2015-06-01 01:12:32 UTC
Please check this issue that is the playback halts in the case3.
The issue has reproduced 100%.

If I operate seeking during listening to 1.flac, the issue doesn't occur.
If I doesn't opeate seeking during listening to 2.flac, the issue doesn't occur.

Why does the issue occur in the case3?

The log of "gstaudiobasesink.c:1605:gst_audio_base_sink_get_alignment:<alsasink0>[00m Unexpected discontinuity in audio timestamps of +0:00:32.508185941, resyncing" is outputed when the issue occur.
Comment 2 Tim-Philipp Müller 2015-06-05 09:19:02 UTC
*** Bug 750437 has been marked as a duplicate of this bug. ***
Comment 3 matsumura 2015-06-08 00:29:18 UTC
Created attachment 304745 [details] [review]
the patch to fix the invalid base time

I also attached the patch here.

When I applied this patch, the playback didn't halt. 

Please refer and comment to this patch and the bug.
Comment 4 matsumura 2015-06-08 05:50:59 UTC
I have tested playing various kinds of sound files and checked after seek with gapless playing.
I seem there is no problem on playing ALAC, FLAC and WAVE.

But white noise occurs on playing AIFF files after the song changes to the next.

If I change gst-plugins-bad version 1.4.5 to 1.4.3, the noise problem doesn't occur.
Which does the bug exist in my patch or gst-plugins-bad-1.4.5?
Comment 5 matsumura 2015-06-08 09:15:43 UTC
>I have tested playing various kinds of sound files and checked after seek with >gapless playing.
>I seem there is no problem on playing ALAC, FLAC and WAVE.
Sorry for not explaining enough.

I have tried with the patch of attachment 304745 [details] [review].
Comment 6 Sebastian Dröge (slomo) 2015-06-08 11:50:18 UTC
So you mean with that patch all problems are solved for you? Can you also test with latest GIT master and your patch?
Comment 7 matsumura 2015-06-09 00:33:29 UTC
I have tried the patch on gstreamer 1.4.5 and plugins 1.4.5.
The problem of 'the playback halts after seek with gapless' is solved.
But applying the patch causes white noise on AIFF playing with gst-plugins-bad-1.4.5.

I'll test with latest GIT master.
Comment 8 matsumura 2015-06-11 05:21:53 UTC
I have tested with 1.5.1.

After seeking after 2 numbers, it becomes dumb during several seconds, and timeStamp is stopped with gapless.

The issue was solved after removing the following patch. 
http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/gst/playback/gststreamsynchronizer.c?id=a73631a29dccdc123a0072c2b7f9a594d1c6222a

And the issue of case 3 in description was solved after applying the patch of attachment 304745 [details] [review].

But white noise occurs on playing AIFF files after the song changes to the next.
The noise issue solved after changing gst-plugins-bad version 1.5.1 to 1.4.3.
Comment 9 Sebastian Dröge (slomo) 2015-06-11 08:17:33 UTC
Ok, so with GIT master and the patch here everything was solved for you? Can you update the patch so that it cleanly applies to latest GIT master?
Comment 10 matsumura 2015-06-11 11:01:48 UTC
No. There are two issue.

1.The following patch causes seek problem.
To solve it, it is nessesary to remove the patch.
http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/gst/playback/gststreamsynchronizer.c?id=a73631a29dccdc123a0072c2b7f9a594d1c6222a
May I update my patch to include removing this patch ?


2.White noise occurs on playing AIFF files after the song changes to the next.
I suppose that there is a regression later then gst-plugins-bad-1.4.3.
But I don't still get to the bottom of a problem.
Comment 11 Sebastian Dröge (slomo) 2015-06-11 11:18:46 UTC
(In reply to matsumura from comment #10)
> No. There are two issue.
> 
> 1.The following patch causes seek problem.
> To solve it, it is nessesary to remove the patch.
> http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/gst/playback/
> gststreamsynchronizer.c?id=a73631a29dccdc123a0072c2b7f9a594d1c6222a
> May I update my patch to include removing this patch ?

No, see the linked bug report in that commit. That commit fixed another problem.

Best would be to update your patch in a way that keeps the current code and only additionally fixes your problem, while keeping that other bug fixed.

> 2.White noise occurs on playing AIFF files after the song changes to the
> next.
> I suppose that there is a regression later then gst-plugins-bad-1.4.3.
> But I don't still get to the bottom of a problem.

Ok, that should be something completely separate that we can handle in a different bug :)
Comment 12 matsumura 2015-06-12 08:35:09 UTC
Ok, I see.
I'll try.

But I have one question.
Do you think that the patch take gapless playing into account?
Comment 13 Sebastian Dröge (slomo) 2015-06-12 08:39:50 UTC
Yes, I tested my patch with the gapless playback example:
gst-plugins-base/tests/icles/playback/play7.c
Comment 14 matsumura 2015-06-17 01:13:23 UTC
Created attachment 305445 [details]
test7 added seek command

I have checked gst-plugins-base/tests/icles/playback/play7.c with latest GIT master.It's Ok with the gapless playback.

But a problem has occured with seeking.I added the seek command in test7.See attach the file "test7a_seekable.c".It can build with gst-play-kb.c and gst-play-kb.h in tools folder.

The problem is same as comment8. 

"A sound doesn't come out and timestamp is stopped during several seconds after seeking later than 2nd songs."

You can also see the problem with "gst-play-1.0 -gapless".
Comment 15 Sebastian Dröge (slomo) 2015-06-22 17:02:49 UTC
The problem here is that we have to reset the group_start_time after a flush. Unfortunately the group_start_time is per element, flushing per stream. Let's see...
Comment 16 Sebastian Dröge (slomo) 2015-06-22 18:28:14 UTC
Seems that this specific problem is solved, now in general gapless playback of audio/video and audio/video files is broken.

commit 203b635d0c03d2b7d803b072a701af9696907d57
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Mon Jun 22 20:17:56 2015 +0200

    streamsynchronizer: Reset group start time when flushing
    
    We reset the group start time to the running time of the start of the other
    streams that are not flushed. This fixes seeking in gapless mode after the
    first track has played.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=750013
Comment 17 Sebastian Dröge (slomo) 2015-06-22 18:57:57 UTC
commit bd508a343f1ee729a71fecce9df18b4321aea2ee
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Mon Jun 22 20:54:18 2015 +0200

    streamsynchronizer: Don't wait for sparse streams when doing stream switches
    
    Their stream-start event might come a bit later, like just before the first
    buffer... and queues might run full before that happens.



With this also my audio/video testcase seems to work.
Comment 18 Mathieu Duponchelle 2016-01-09 03:44:48 UTC
Created attachment 318559 [details] [review]
streamsynchronizer: Ignore flushing streams [..]

[..] when resetting group start time. In GES, we are usually connected
to the streamsynchronizer on one audio and one video pad.

When seeking the timeline, both nlecompositions often output their flush_start
before any of them has output its flush_stop.

The current code, when receiving the first flush stop was using the
running time of the start of the second composition, which could
be pretty much anything, and means nothing at that point.

This patch is thread-safe, as STREAM_SYNCHRONIZER_LOCK is taken
both when setting flushing and when checking it.
Comment 19 Mathieu Duponchelle 2016-01-09 04:01:27 UTC
Wanted to play with pitivi, ended up spending 4 hours debugging this, fun times.
This was breaking GES pretty badly. Another solution would be to reset the stream's segment at flush_start, not sure which solution is cleaner.

On a side note it still feels like GES sits on top of a very fragile pyramid :/

We ought to document a bit more formally some concepts, I know docs/design exist, but for example I wanted to check what the "reset_time" parameter of new_flush_stop was exactly meant for, went there -> https://developer.gnome.org/gstreamer/stable/gstreamer-GstEvent.html#gst-event-new-flush-stop (which is where the average developer will go first), and the documentation for that parameter is pretty useless. (same goes for the doc for GstSegment , the really tricky and unintuitive fields are left as an exercise for the imagination of the reader (base and offset))

It would also be nice to have a bullet list of what exactly should happen in an element that receives a given event.

And yeah I know I could do it, but I think these core concepts should be documented by the core maintainers :)

Anyway, this is borderline ranting and not really relevant here, I'll try to come up with some more constructive solutions on IRC :)
Comment 20 Tim-Philipp Müller 2016-01-10 12:00:19 UTC
All unit tests in ges seem to pass for me, if it was breaking ges badly, it might be nice to add some additional tests there. (On a side note, the docs you link are for 1.2.4 and outdated, but please file any docs issues as separate bug. The GstSegment docs at least have improved since.)
Comment 21 Sebastian Dröge (slomo) 2016-01-10 12:17:01 UTC
Can you explain what exactly this patches and why/how? A test would be good for that (and if it's just a manual test to reproduce the problem), also we should make sure that gapless playback still works, including switching between audio/video/audio-video files and seeking (for which there is also not test currently).


For flush-stop's reset_time parameter. If it's TRUE, the segment should be reset. If it's FALSE it should be kept and just flushing/unblocking should happen. (almost?) nothing handles this currently and just assumes it's always TRUE though.

For the segment docs, check the latest docs on the GStreamer website and docs/design/part-synchronization. The documentation was recently improved.


And last, please create a new bug report for this and just link to this one. Otherwise the target milestone field for this bug report will become meaningless :) Also mark the new bug a blocker for 1.7.2 for now.
Comment 22 Sebastian Dröge (slomo) 2016-01-10 12:18:53 UTC
Comment on attachment 318559 [details] [review]
streamsynchronizer: Ignore flushing streams [..]

That said, it makes sense to ignore flushing streams here probably. I just wonder if there are also more places where they should be ignored.
Comment 23 Mathieu Duponchelle 2016-01-10 19:24:43 UTC
Review of attachment 318559 [details] [review]:

Proposed in https://bugzilla.gnome.org/show_bug.cgi?id=760408 , rejecting here