GNOME Bugzilla – Bug 787300
basesink: Add property to handle seamless-seek by sink
Last modified: 2018-11-03 12:42:24 UTC
There seems to no way to set segment.rate immediately in pipeline without flush. To mimic the feature, there might be 2 options but seems ugly for me. - Reduce queue size in pipeline as much as possible and use non-flush seek. - Adjust clock maybe?? Another option is that make use of step event but originally step event is not for this feature. So we need more pretty way to change rate immediately. One of use case of this rate change is, Youtube rate change which is perfectly seamless.
Created attachment 359170 [details] [review] basesink: Add property to handle seamless-seek by sink Add "enable-seamless-seek" property to handle some special seek events which satisfy following conditions * non-flush * positive rate * start_type is none In that case, sink will not forward the seek event to upstream and handle it by self (only modify internal segment). Default FALSE This will be very useful in case of seamless rate change (without flush) such as Youtube trick play. https://bugzilla.gnome.org/show_bug.cgi?id=787300
(In reply to Seungha Yang from comment #0) > There seems to no way to set segment.rate immediately in pipeline without > flush. > To mimic the feature, there might be 2 options but seems ugly for me. > - Reduce queue size in pipeline as much as possible and use non-flush seek. > - Adjust clock maybe?? > > Another option is that make use of step event but originally step event is > not for this feature. So we need more pretty way to change rate immediately. > > One of use case of this rate change is, Youtube rate change which is > perfectly seamless. As I know, if you are using flush-stop with no reset-time, start-time is not reset and we would get valid base-time of clock.
Thought about that issue some time ago. The way I see it, in order to get fast trick-mode switching we need to solve two issues: [1] Segment changes that do not "break" the stream, but only change timing information, should be handled as quickly/smoothly as possible in various elements (demuxers, parsers, decoders, ..). These would be segment changes that you mention (ex: only change would the rate field). I have this feeling quite a few elements reconfigure themselves when they get a new segment, instead of just checking whether they need to store/forward the updated segment. This would need to be checked/fixed. [2] Applying such (seek) changes as quickly as possible throughout the pipeline. The problem with your proposal is that the segment is no longer consistent throughout various elements. Elements upstream that use gst_segment_to_running_time() will return inconsistent values. That alone would break QoS handling (where the diff value is provided in running time). Two solutions: 2.1. Having an out-of-band segment event that could propagate such segment updates 2.2. Having an "eventually consistent" segment propagation. 2.1: Pros: changes would apply as quickly as possible Cons: Requires a new API 2.2: The idea would be to have the sink apply the rate change *before* sending the seek event upstream. If the seek fails, it obviously resets it to the previous segment. Otherwise it keeps it. Pros: Change happens as fast as can be, no new API needed, every element gets the segment update. The pipeline is "eventually" consistent from a segment pov Cons: That small window where segments are inconsistent, but could be worked around. Thoughts ?
I'd like to drop some thought too. In general the proposed approach made sense. But there is some difficult aspects considering our distributed model of segments. 2.1 An out-of-band segment event (non-serialized) it feasible, though every single element will need to be updated. Were things gets hairy is for baseclass that offers direct access to the segment, assuming it will always be protected by the stream-lock (single threaded access). Good care will be needed for the locking. I know you understand this pretty well (probably better then me in some ways), but the way it's written make us believe that only the "rate" field need to be updated (this apply to rate change only in the seek of course). So just to set the expectation, elements driving the pipeline will need for push forward the base of the new segment too. This is because the rate for the portion of time that passed was at at different rate then the portion we are going to play. Running Time is scaled with the rate, and base is a value of running time. For other elements, forwarding is all you have to do. So I expect loads or trivial patches. I hope base-transform and other base classes will be able to hide this up well. 2.2 Doing it that way around is hard, since each element may endup with a different base (they have different buffers in hand). You expect the new base to adjust to where the demuxer is, so that if frame-dropping is needed, it is smooth out by having some data queued. It's much simpler to add a new event, as you can pass the segment that downstream can trivially use in most cases. So to me, the Cons is quite big. It will be extra hard to debug, but might work as well.
Dear All. Your comments are really helpful to us. Thanks. I mentioned about "YouTube Trick Play" as following address. (http://gstreamer-devel.966125.n4.nabble.com/Playbin3-can-supports-YouTube-trick-play-td4684619.html) If you are ok, I want to discuss more about it on this bugzilla.
To clarify one point: The type of segment changes we want to handle are those such that: * The data stream in itself isn't broken (i.e. you are not changing the direction and the buffers remain contiguous (i.e. no DISCONT)). * The running time just before and just after the new SEGMENT remains the same > 2.1 > An out-of-band segment event (non-serialized) it feasible, though every > single element will need to be updated. Were things gets hairy is for > baseclass that offers direct access to the segment, assuming it will always > be protected by the stream-lock (single threaded access). Good care will be > needed for the locking. Well spotted, so 2.1 also means locking issues everywhere. Oh, and I forgot another con: this breaks apart completely with 3rd party elements which don't understand this new event yet. > > I know you understand this pretty well (probably better then me in some > ways), but the way it's written make us believe that only the "rate" field > need to be updated (this apply to rate change only in the seek of course). > So just to set the expectation, elements driving the pipeline will need for > push forward the base of the new segment too. This is because the rate for > the portion of time that passed was at at different rate then the portion we > are going to play. Running Time is scaled with the rate, and base is a value > of running time. For other elements, forwarding is all you have to do. So I > expect loads or trivial patches. I hope base-transform and other base > classes will be able to hide this up well. Yes, in any situation the seek handler would have to properly update the base value to guarantee running_time continuity. This is part of what I meant in issue [1] in comment #3. > > 2.2 > Doing it that way around is hard, since each element may endup with a > different base (they have different buffers in hand). You expect the new > base to adjust to where the demuxer is, so that if frame-dropping is needed, > it is smooth out by having some data queued. It's much simpler to add a new > event, as you can pass the segment that downstream can trivially use in most > cases. So to me, the Cons is quite big. It will be extra hard to debug, but > might work as well. Well .. not *each* element. Since the segment update is in-band and we respect the running_time continuity, that wouldn't be an issue. *BUT* ... I just realized one issue which would break all of this. This is having two different segments when switching. The temporary one in the sink, and the final one which will replace it. Take the following (simplified) example: We play for 10s a stream, then do a switch to 2x rate. The queueing between the demuxer (seek handler) and sink is of 2s. * Playback for 10s Initial segment will result in following values: Position: 0s -> 10s Running time : 0s -> 10s * Create a 2x rate segment * Sink creates a provisional segment with a base of 10s (the current position) and a rate of 2x * Sink sends seek event upstream to demuxer * Demuxer is at a position of 12s, and creates/pushes a new segment with a base of 12s and a rate of 2x * Before the sink receives the new segment, it will receive buffers from 10s to 12s, resulting in the following values: Position : 10s -> 12s Running time : 10s -> 11s * The sink receives the new segment and buffers from 12s, resulting in: Position : 12s -> ... Running time : 12s -> ... *BOOOM* We end up having a "gap" of 1s where playback will be "paused". One way we could circumvent this is to allow seek handlers to provide the base/position they will use, so that the provisional segment that the sink creates ends up being contiguous. But either way it's going to be tricky. Need to think some more about this.
Dear All. I want to explain our current status for YouTube pipeline. This is our YouTube Pipeline. (Not using playbin pipeline) Our Player (App) is feeding element streams to each appsrc. ---------------------------------------------------------------------------- video-appsrc - queue - H/W VDEC - queue - H/W VSINK audio-appsrc - queue - audio parse (optional) - H/W ADEC - queue - H/W ASINK ---------------------------------------------------------------------------- As I know, there is differences with common trick play. It wants to decoding fully and rendering fully regardless of playback rate and without dropping buffers. We think that we could meet the seamless behaviour if we do not flush. Thus, we notify changed segment.rate, segment.base, segment.start and segment.time .. through new custom event with new segment information to each video and audio sink directly. Actually, new segment information contains the current rendered PTS (e.g. current position) by segment.start. I think it is not permitted to adjust start-time and base-time of the pipeline's clock in general case. However, I am not sure either we do adjust clock or not for YouTube trick play. As your concerned, I think it is not easy to handle clock and it needs more discussion. In temporary, we implemented 'prepare_seamless_seek' new API and delegate to each audio/video sink to synchronize buffers, new segment and clock.
Created attachment 360296 [details] [review] render all frames without dropping in positive playback rates Dear All. Please review our solution. We delegated to synchronize buffers, segment and clock to each video/audio sink. Thanks.
Dear All. I want to explain more. And I want to assume that new segment is delivered directly to sink and it is not forwarding to upstream (demux layer). For example, 1) play 10s with 1x. 2) trick with 2x at 10s. 3) trick with 0.25 at 20s. 4) trick with 1.x at 25s. 5) paused while 10s. 6) play Assume clock's absolute time at starting 0s. For 1) * clock(absolute-time): 0s -> 10s * c.start-time: 0s * c.base-time: 0s * running-time: 0s -> 10s For 2) * clock: 11s -> 20s * c.running-time vs b.running-time 11s -> 12s 12s -> 14s 13s -> 16s 14s -> 18s 15s -> 20s 16s -> 22s 17s -> 24s 18s -> 26s 19s -> 28s 20s -> 30s For 3) * clock: 21s -> 25s * c.running-time vs b.running-time 21s -> 30.25s 22s -> 30.50s 23s -> 30.75s 24s -> 31.00s 25s -> 31.25s For 4) * clock: 26s -> 30s * c.running-time vs b.running-time 26s -> 32.25s 27s -> 33.25s 28s -> 34.25s 29s -> 35.25s 30s -> 36.25s For 5) * clock: 31 -> 40s * c.start-time: 30s * c.base-time: 0s * c.running-time: 30s For 6) * clock: 41s -> ... * c.start-time: 30s * c.base-time: 10s * c.running-time: 30s * c.running-time vs b.running-time 31s -> 37.25s My analysis is correct?? As I know, c.running-time is 31s and sink may would expect a buffer with timestamp 31s after paused to playing. I am guessing there will be a jitter. I am not sure how to synchronize correctly between clock, buffer and segment. It would be helpful if you give some advices. Thanks.
Additionally, I am thinking about c.start-time and c.base-time. How about staring and storing c.start-time and c.base-time internally whenever playback rate is changed?? Likes flushing situation. I think that we can update c.start-time and c.base-time by stored internal starttime and basetime when paused to play. I hope it could adjust clock as quiet well. Please review my opinion. Thanks.
Created attachment 360714 [details] [review] basesink: render all frames with gapless rate change
Dear All. I rebased my patch. Let me explain my solution. My purpose is that to update start-time and base-time of current clock whenever playback rate is changed with non-flush in order to behave flush-seek and have valid running time. 1) Post "update-start-time" custom message from basesink to update start-time in a pipeline. 2) Post "reset-time" message from basesink to reset start-time in a pipeline. 3) Post "update-base-time" custom message from basesink to update base-time in a pipeline. I hope this solution may have valid running time whenever playback rate is changed. Please review my patch and check it is acceptable or not. Don't hesitate give any information or advices. Thanks.
-- 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/248.