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 667595 - [0.11] Subtitle segments have zero base
[0.11] Subtitle segments have zero base
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
0.11.x
Other Mac OS
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-01-09 18:07 UTC by Matej Knopp
Modified: 2012-04-30 21:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add GAP event methods to gstreamer (4.05 KB, patch)
2012-01-29 01:55 UTC, Matej Knopp
rejected Details | Review
Add GAP event support to matroska-demux (2.09 KB, patch)
2012-01-29 01:56 UTC, Matej Knopp
needs-work Details | Review

Description Matej Knopp 2012-01-09 18:07:15 UTC
Running time can't be used to synchronize subtitles in 0.11 because the segments don't seem to have base set. 

The start field keeps changing with every segment but base remains 0. What is the proper way of fixing that? Should every demuxer be responsible for setting the proper base value in subsequent segments?
Comment 1 Matej Knopp 2012-01-09 18:23:04 UTC
According to Tim on IRC it's not entirely clear yet how subtitle streams are supposed to work in 0.11, would be really nice if someone could chime in and figure it out.
Comment 2 Sebastian Dröge (slomo) 2012-01-09 18:32:11 UTC
Last time I thought about this, the answer was to have demuxers (and whatever) set the correct values for the base field of the segments for filler segment events (which is the problem here, right?).
Comment 3 Matej Knopp 2012-01-09 19:06:24 UTC
Right. Every segment event comes with base = 0 and increasing start, which results in wrong running time (position - start + base). 

So just to be clear, 0.11 will use same approach as 0.10 in regard to sparse stream? (I'm asking because the 0.11 Wiki page mentions rethinking how sparse streams work - "currently using newsegment updates, should think of something else (events? dummy buffers?)")
Comment 4 Sebastian Dröge (slomo) 2012-01-09 21:10:23 UTC
Something else would be nice to have, sure. But I don't think anybody had a better idea yet and it's already very late and 1.0 is near.
Comment 5 Mark Nauwelaerts 2012-01-10 10:31:20 UTC
Still not clear on why something else would be nice to have (what is the problem to be solved with the current approach, which does seem to work ?)

Would not care much for dummy buffers, so that would come down to event(s) with varying degree of complexity.  For muxing purposes, it would have to convey some kind of (start)time, which brings it pretty close to a segment event which might then as well be re-used.  Semantics also match pretty well since a segment event does actually convey subsequent data is expected to be within indicated boundary.

The missing and problematic part in 0.11 afaics is that it would be useful to (optionally) know that the event concerns a (sparse) update, i.e. like the 0.10 'update' field, which is needed here and there on occasion (and conveniently ignored/disabled for now while porting e.g. dvdsubdec).  The 'update' field could be added to the segment event or a reason to come up with a separate event.

Similarly, either newsegment or (that same) separate event could be used to arrange for pre-roll in sinks (like proposed 'soft-eos' or whatever as intended in bug #633700, which did not quite make it through for not so clear or administrative reasons).
Comment 6 Sebastian Dröge (slomo) 2012-01-10 10:47:25 UTC
Bug #600648 has something to signal sparse streams, but that's what I meant. We need something to signal if a stream is sparse or not in different places.

What is the problem with not knowing if the segment event is an update to the previous segment or not?
Comment 7 Mark Nauwelaerts 2012-01-10 11:02:03 UTC
(In reply to comment #6)
> Bug #600648 has something to signal sparse streams, but that's what I meant. We
> need something to signal if a stream is sparse or not in different places.

That would then be in addition to the segment event sending, since that one still needs to update timing info as needed by e.g. GstCollectPads2.

> 
> What is the problem with not knowing if the segment event is an update to the
> previous segment or not?

See for example dvdsubdec as mentioned above.  One might argue whether or not it is really needed there, but would prefer not to be caught with being able to convey less info than was possible in 0.10 without considering it a lot more (rather than solving it by disabling).

Also, seeing an event with update=TRUE coming true might then as well signal that the stream is sparse.
Comment 8 Sebastian Dröge (slomo) 2012-01-10 11:51:49 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > Bug #600648 has something to signal sparse streams, but that's what I meant. We
> > need something to signal if a stream is sparse or not in different places.
> 
> That would then be in addition to the segment event sending, since that one
> still needs to update timing info as needed by e.g. GstCollectPads2.

Agreed

> > What is the problem with not knowing if the segment event is an update to the
> > previous segment or not?
> 
> See for example dvdsubdec as mentioned above.  One might argue whether or not
> it is really needed there, but would prefer not to be caught with being able to
> convey less info than was possible in 0.10 without considering it a lot more
> (rather than solving it by disabling).

You should be able to infer the information (if this segment is an update or not) from the previous and the new segment. If the base is still the same you have an update. Am I missing something?

> Also, seeing an event with update=TRUE coming true might then as well signal
> that the stream is sparse.

That's not true in all cases, there could be an segment update because the stream now has a new stop position for example.
Comment 9 Mark Nauwelaerts 2012-01-10 12:09:57 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > What is the problem with not knowing if the segment event is an update to the
> > > previous segment or not?
> > 
> > See for example dvdsubdec as mentioned above.  One might argue whether or not
> > it is really needed there, but would prefer not to be caught with being able to
> > convey less info than was possible in 0.10 without considering it a lot more
> > (rather than solving it by disabling).
> 
> You should be able to infer the information (if this segment is an update or
> not) from the previous and the new segment. If the base is still the same you
> have an update. Am I missing something?

The fix to this bug would have to modify base along with increasing (modifying) start time (afaik), so that would also be an update (in 0.10) but with changing base.  It would feel only the sender 'knows' this is really some new segment (concerning other data etc) or has some new/updated info w.r.t. previous segment.

> 
> > Also, seeing an event with update=TRUE coming true might then as well signal
> > that the stream is sparse.
> 
> That's not true in all cases, there could be an segment update because the
> stream now has a new stop position for example.

True, so one might wonder whether one should signal the 'stream' to be sparse or the 'segment' to be sparse, in which case another (optional) field in the segment event might convey that.  But that might be a matter of nitpick semantics and don't mind either way.
Comment 10 Sebastian Dröge (slomo) 2012-01-10 12:19:20 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #6)
> > > > What is the problem with not knowing if the segment event is an update to the
> > > > previous segment or not?
> > > 
> > > See for example dvdsubdec as mentioned above.  One might argue whether or not
> > > it is really needed there, but would prefer not to be caught with being able to
> > > convey less info than was possible in 0.10 without considering it a lot more
> > > (rather than solving it by disabling).
> > 
> > You should be able to infer the information (if this segment is an update or
> > not) from the previous and the new segment. If the base is still the same you
> > have an update. Am I missing something?
> 
> The fix to this bug would have to modify base along with increasing (modifying)
> start time (afaik), so that would also be an update (in 0.10) but with changing
> base.  It would feel only the sender 'knows' this is really some new segment
> (concerning other data etc) or has some new/updated info w.r.t. previous
> segment.

Yes, start and base have to be increased for fixing this bug (base is essentially the same as accum in 0.10 but the accumulator changes were done implicitely).

The receiver of the segment really seems to have no way to know if this is an update or not from the information inside the segment, you're right. But does it have to know? If a new stream starts the next buffer must have the DISCONT flag set

> > > Also, seeing an event with update=TRUE coming true might then as well signal
> > > that the stream is sparse.
> > 
> > That's not true in all cases, there could be an segment update because the
> > stream now has a new stop position for example.
> 
> True, so one might wonder whether one should signal the 'stream' to be sparse
> or the 'segment' to be sparse, in which case another (optional) field in the
> segment event might convey that.  But that might be a matter of nitpick
> semantics and don't mind either way.

I'm fine with adding that to the segment but really don't care either ;)
Comment 11 Mark Nauwelaerts 2012-01-10 12:35:17 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (In reply to comment #7)
> > > > (In reply to comment #6)
> > > > > What is the problem with not knowing if the segment event is an update to the
> > > > > previous segment or not?
> > > > 
> > > > See for example dvdsubdec as mentioned above.  One might argue whether or not
> > > > it is really needed there, but would prefer not to be caught with being able to
> > > > convey less info than was possible in 0.10 without considering it a lot more
> > > > (rather than solving it by disabling).
> > > 
> > > You should be able to infer the information (if this segment is an update or
> > > not) from the previous and the new segment. If the base is still the same you
> > > have an update. Am I missing something?
> > 
> > The fix to this bug would have to modify base along with increasing (modifying)
> > start time (afaik), so that would also be an update (in 0.10) but with changing
> > base.  It would feel only the sender 'knows' this is really some new segment
> > (concerning other data etc) or has some new/updated info w.r.t. previous
> > segment.
> 
> Yes, start and base have to be increased for fixing this bug (base is
> essentially the same as accum in 0.10 but the accumulator changes were done
> implicitely).
> 
> The receiver of the segment really seems to have no way to know if this is an
> update or not from the information inside the segment, you're right. But does
> it have to know? If a new stream starts the next buffer must have the DISCONT
> flag set

But one can also have DISCONT without having a new stream or segment.  Furthermore, the semantics of DISCONT on a sparse stream are a bit shady in that (almost) every subsequent buffer is possibly discontinuous w.r.t. previous one.  And even otherwise, it is not clear/certain that a plugin's response/actions when about to deal with a (really) new segment (exactly) match those that are appropriate for a (minor?) stream discontinuity.  The former might involve finishing up some delayed stuff from current segment, the latter might involve discarding such anyway (?)
Comment 12 Sebastian Dröge (slomo) 2012-01-27 15:43:49 UTC
The demuxer is supposed to update the segment base as necessary but use the new GAP event as a replacement for the filler segments. The GAP event should have a start time and duration (or stop time) but the API is still missing.
Comment 13 Matej Knopp 2012-01-27 16:01:35 UTC
How is this supposed to work? Are the gap events supposed to go between buffers? Are the regular segments still necessary, or is will it be enough to send regular segment once (after seek) just like with other streams?
Comment 14 Sebastian Dröge (slomo) 2012-01-27 16:05:55 UTC
Only a single segment as for other streams too. And the GAP events are basically used like buffers and are sent regularily (like every 0.5s like the filler segments) when there's a (larger) GAP between buffers
Comment 15 Matej Knopp 2012-01-29 01:55:43 UTC
Created attachment 206343 [details] [review]
Add GAP event methods to gstreamer
Comment 16 Matej Knopp 2012-01-29 01:56:46 UTC
Created attachment 206344 [details] [review]
Add GAP event support to matroska-demux
Comment 17 Sebastian Dröge (slomo) 2012-01-30 09:17:00 UTC
Comment on attachment 206343 [details] [review]
Add GAP event methods to gstreamer

This was already done by Tim but his API is slightly different. There's a duration instead of a stop time to be more consistent with GstBuffer

commit 068d9ecf5ac6b16cfa5c74db39e029e35a24cc96
Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
Date:   Fri Jan 27 18:56:01 2012 +0000

    event: add constructor and parse function for new GAP event
    
    (Whatever you do, don't mention the filler event.)
Comment 18 Sebastian Dröge (slomo) 2012-01-30 09:18:49 UTC
Comment on attachment 206344 [details] [review]
Add GAP event support to matroska-demux

Slightly different GstEvent API now.

Also this needs handling in basesink at least (and in all other elements doing synchronization... textoverlay, muxers, ...)
Comment 19 Matej Knopp 2012-04-30 21:03:16 UTC
Closing as obsolete as the gap event has already been added.