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 698050 - tsdemux: seeking doesn't even work in pull mode
tsdemux: seeking doesn't even work in pull mode
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal blocker
: 1.1.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 680285 700127 700820 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-04-15 10:41 UTC by Tim-Philipp Müller
Modified: 2013-06-12 08:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch that fixes seeking in pull mode (4.96 KB, patch)
2013-05-23 07:58 UTC, Josep Torra Valles
none Details | Review
patch that fixes seeking in pull mode (6.13 KB, patch)
2013-05-24 09:03 UTC, Josep Torra Valles
none Details | Review

Description Tim-Philipp Müller 2013-04-15 10:41:45 UTC
Didn't even work in 0.10 apparently.

Should maybe at least be disabled then.
Comment 1 Tim-Philipp Müller 2013-04-16 08:53:31 UTC
What happens is that when we do a flushing seek:

 - gst_ts_demux_flush() will re-init
     demux->segment to UNDEFINED

 - then gst_ts_demux_do_seek() will
     use the newly-unset demux->segment
     as basis for the post-seek segment.

 -> fail (GStreamer-CRITICAL **: gst_segment_do_seek: assertion `segment->format == format' failed)
Comment 2 Tim-Philipp Müller 2013-04-21 10:34:45 UTC
Also see bug #675132. However, the problems I'm seeing at the moment are more fundamental. Flushing the observations might also not be helpful.
Comment 3 Edward Hervey 2013-04-26 15:06:14 UTC
That specific issue was introduced by the segment reset in the following commit:

commit 3d012665f06797cd2cd3c7c2f4df7144319b44ce
Author: Josep Torra <n770galaxy@gmail.com>
Date:   Tue Nov 13 22:40:25 2012 +0100

    tsdemux: forward upstream time segments after flushes
    
    Also reset segment info and drop the segment event when demuxer is
    flushed.
    Restore demuxer segment with the info stored in base when demuxer is
    going to push data again if needed.
    Drop code to recover the segment info from base in the initial program
    becauses it's superseded by the new code.
Comment 4 Tim-Philipp Müller 2013-04-26 15:41:00 UTC
Yes, I talked with him about that the other day on IRC, apparently it had to do with timeshifting:


#gstreamer 23-04-2013 12:50:01

   tim: AD-N770, do you remember what the rationale was for http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=150376efe476f4b9ea6c6cef54af1399cf6f478e ?
   tim: AD-N770, flushing observations doesn't exactly help with seeking, if I'm not mistaken
 josep: I don't remember now, was something related with tsdemux and the leak on _dispose
 slomo: AD-N770: so only a leak, no functional reason?
 josep: I've the feeling that was also a functional reason but I can't remember now
 josep: I mean that the feeling I've is that the leak on dispose was found as a side effect 
 josep: but I can't remember the exact reason for it was needed
 josep: __tim: as for the context of the moment I've commited it I think that was somehow related to on the use case that the time seek is handled upstream like in my timeshift element or hlsdemux
   tim: AD-N770, and I remember you fixed that nasty crash where it cleaned up stuff in FLUSH_START instead of STOP, but I didn't see how those commits were related to that
 josep: well at that time I was trying to make the timeshift element work with tsdemux in 1.0
 josep: that triggered the raising of multiple issues
 josep: my gut feeling is that having previous observations after seek when it was handled upstream caused something nasty with the timestamps or the segment
 josep: well dvbsrc is the exception
 josep: ah, yes it's live
 josep: but I think that demuxer always create de segment
   tim: AD-N770, even if upstream is TIME?
 josep: you should ask bilboed, but I think that is recreated in demuxer always
   tim: hrm, maybe it creates a new one based on the input values then, which would be ok
   tim: anyway, I now know what to look out for if I poke at it, thanks
Comment 5 Edward Hervey 2013-04-29 07:55:10 UTC
*** Bug 680285 has been marked as a duplicate of this bug. ***
Comment 6 Tim-Philipp Müller 2013-05-11 17:29:10 UTC
*** Bug 700127 has been marked as a duplicate of this bug. ***
Comment 7 lips.john 2013-05-11 21:59:30 UTC
Tim let me know if there is something I can do to help.  I have the time and motivation to get this problem fixed, although I may need a little coaching.
Comment 8 Tim-Philipp Müller 2013-05-13 12:42:02 UTC
Marking this as (soft) blocker, since it's really quite important.
Comment 9 Sebastian Dröge (slomo) 2013-05-22 07:31:00 UTC
*** Bug 700820 has been marked as a duplicate of this bug. ***
Comment 10 Josep Torra Valles 2013-05-23 07:56:19 UTC
(In reply to comment #4)
> Yes, I talked with him about that the other day on IRC, apparently it had to do
> with timeshifting:
> 
> 
> #gstreamer 23-04-2013 12:50:01
> 
>    tim: AD-N770, do you remember what the rationale was for
> http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=150376efe476f4b9ea6c6cef54af1399cf6f478e
> ?
>    tim: AD-N770, flushing observations doesn't exactly help with seeking, if
> I'm not mistaken
>  josep: I don't remember now, was something related with tsdemux and the leak
> on _dispose
>  slomo: AD-N770: so only a leak, no functional reason?
>  josep: I've the feeling that was also a functional reason but I can't remember
> now
>  josep: I mean that the feeling I've is that the leak on dispose was found as a
> side effect 
>  josep: but I can't remember the exact reason for it was needed
>  josep: __tim: as for the context of the moment I've commited it I think that
> was somehow related to on the use case that the time seek is handled upstream
> like in my timeshift element or hlsdemux
>    tim: AD-N770, and I remember you fixed that nasty crash where it cleaned up
> stuff in FLUSH_START instead of STOP, but I didn't see how those commits were
> related to that
>  josep: well at that time I was trying to make the timeshift element work with
> tsdemux in 1.0
>  josep: that triggered the raising of multiple issues
>  josep: my gut feeling is that having previous observations after seek when it
> was handled upstream caused something nasty with the timestamps or the segment
>  josep: well dvbsrc is the exception
>  josep: ah, yes it's live
>  josep: but I think that demuxer always create de segment
>    tim: AD-N770, even if upstream is TIME?
>  josep: you should ask bilboed, but I think that is recreated in demuxer always
>    tim: hrm, maybe it creates a new one based on the input values then, which
> would be ok
>    tim: anyway, I now know what to look out for if I poke at it, thanks

I think that the reason was that when the seek in time was handled by the upstream element a wrong offset was applied on the produced timestamps due previous observations causing the the pts not being related anymore on the upstream provided segment event.
Comment 11 Josep Torra Valles 2013-05-23 07:58:02 UTC
Created attachment 245104 [details] [review]
patch that fixes seeking in pull mode

This patch fixes the critical triggered on pull mode seeks.
Also keeps the observations on pull mode seeks.
Comment 12 Josep Torra Valles 2013-05-24 09:03:32 UTC
Created attachment 245213 [details] [review]
patch that fixes seeking in pull mode
Comment 13 Edward Hervey 2013-06-12 06:06:53 UTC
There are still some minor isues to fix (doing a seeking where you only change rate seems to result in glitches), but there was nothing blocking it.

Thanks !


commit 28a2902a7b6601785c9237dfb6218a68f1845083
Author: Josep Torra <n770galaxy@gmail.com>
Date:   Fri May 24 10:59:59 2013 +0200

    tsdemux: fixes seeking in pull mode
    
    Preserve the current segment and observations in pull mode seeks with
    flushing.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=698050