GNOME Bugzilla – Bug 764707
segment: Modifiy inside segment condition
Last modified: 2017-01-31 15:09:46 UTC
segment: Modifiy inside segment condition There is a special case that segment_start == segment_stop == start. It's inside segment, but current old code did not consider it.
Created attachment 325516 [details] [review] segment: Modifiy inside segment condition
This approach similar to commit 76a55f 2009-08-11 gstsegment: Actually start==stop==segment_start is inside the segment
There is a mp4 file which have different track duration between video and audio. - Video track duration: 48 sec - Audio track duration: 29 sec For the file, seek to over 29 sec is failure always. Detailed description is as follows. - 1. Seek to over 29 sec qtdemux figure out keyframe offset, segment, and etc for video and audio Then, segments for video and audio are different <For Video> 0:00:03.951523490 22988 0x7f4a201181e0 DEBUG GST_EVENT gstpad.c:5531:gst_pad_send_event_unchecked:<multiqueue0:sink_0> have event type segment event: 0x7f4a1803e040, time 99:99:99.999999999, seq-num 1024, GstEventSegment, segment=(GstSegment)"GstSegment, flags=(GstSegmentFlags)GST_SEGMENT_FLAG_RESET, rate=(double)1, applied-rate=(double)1, format=(GstFormat)GST_FORMAT_TIME, base=(guint64)0, offset=(guint64)0, start=(guint64)30215692499, stop=(guint64)48048000000, time=(guint64)30215692499, position=(guint64)30215692499, duration=(guint64)18446744073709551615;"; <For Audio> 0:00:03.951859403 22988 0x7f4a201181e0 DEBUG GST_EVENT gstpad.c:5531:gst_pad_send_event_unchecked:<multiqueue0:sink_1> have event type segment event: 0x1cfc640, time 99:99:99.999999999, seq-num 1024, GstEventSegment, segment=(GstSegment)"GstSegment, flags=(GstSegmentFlags)GST_SEGMENT_FLAG_RESET, rate=(double)1, applied-rate=(double)1, format=(GstFormat)GST_FORMAT_TIME, base=(guint64)0, offset=(guint64)0, start=(guint64)29696000000, stop=(guint64)29696000000, time=(guint64)30215692499, position=(guint64)29696000000, duration=(guint64)18446744073709551615;"; Note that segment.start == segment.stop for audio. - 2. After flush start - stop <video sink element get segment -> commit state then wait playing> 0:00:03.965053464 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:3166:gst_base_sink_default_event:<xvimagesink0> configured segment time segment start=0:00:30.215692499, offset=0:00:00.000000000, stop=0:00:48.048000000, rate=1.000000, applied_rate=1.000000, flags=0x01, time=0:00:30.215692499, base=0:00:00.000000000, position 0:00:30.215692499, duration 99:99:99.999999999 0:00:04.006421592 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:3408:gst_base_sink_chain_unlocked:<xvimagesink0> got times start: 0:00:30.215692499, end: 0:00:30.238541666 0:00:04.006479980 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:1957:gst_base_sink_get_sync_times:<xvimagesink0> got times start: 0:00:30.215692499, stop: 0:00:30.238541666, do_sync 1 0:00:04.006498002 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:2256:gst_base_sink_do_preroll:<xvimagesink0> prerolling object 0x7f4a1c58b5d0 0:00:04.006529952 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:945:gst_base_sink_set_last_buffer_unlocked:<xvimagesink0> setting last buffer to 0x7f4a1c58b5d0 0:00:04.006540546 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:2279:gst_base_sink_do_preroll:<xvimagesink0> preroll buffer 0:00:30.215692499 0:00:04.007878708 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:1502:gst_base_sink_commit_state:<xvimagesink0> commiting state to PAUSED 0:00:04.007920258 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:1527:gst_base_sink_commit_state:<xvimagesink0> posting PAUSED state change message 0:00:04.007955615 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:1533:gst_base_sink_commit_state:<xvimagesink0> posting async-done message 0:00:04.007988127 22988 0x7f4a1c51c140 DEBUG basesink gstbasesink.c:2209:gst_base_sink_wait_preroll:<xvimagesink0> waiting in preroll for flush or PLAYING <audio sink element get segment -> get gap event (generated by streamsynchronizer)> This is the problem happen point. audio sink element drop the gap event because start (of gap) is identical to segment.stop So, audio sink element did not commit state 0:00:03.952646266 22988 0x7f4a1c579190 DEBUG basesink gstbasesink.c:3166:gst_base_sink_default_event:<alsasink1> configured segment time segment start=0:00:29.696000000, offset=0:00:00.000000000, stop=0:00:29.696000000, rate=1.000000, applied_rate=1.000000, flags=0x01, time=0:00:30.215692499, base=0:00:00.000000000, position 0:00:29.696000000, duration 99:99:99.999999999 0:00:03.952902581 22988 0x7f4a1c579190 DEBUG GST_EVENT gstpad.c:5531:gst_pad_send_event_unchecked:<alsasink1:sink> have event type gap event: 0x1bb2b20, time 99:99:99.999999999, seq-num 1046, GstEventGap, timestamp=(guint64)29696000000, duration=(guint64)18446744073709551615; 0:00:03.952914669 22988 0x7f4a1c579190 DEBUG basesink gstbasesink.c:3231:gst_base_sink_event:<alsasink1> received event 0x1bb2b20 gap event: 0x1bb2b20, time 99:99:99.999999999, seq-num 1046, GstEventGap, timestamp=(guint64)29696000000, duration=(guint64)18446744073709551615; 0:00:03.952941688 22988 0x7f4a1c579190 DEBUG basesink gstbasesink.c:1922:gst_base_sink_get_sync_times:<alsasink1> Got Gap time 0:00:29.696000000 duration 99:99:99.999999999 0:00:03.952957053 22988 0x7f4a1c579190 DEBUG basesink gstbasesink.c:1957:gst_base_sink_get_sync_times:<alsasink1> got times start: 0:00:29.696000000, stop: 99:99:99.999999999, do_sync 1 0:00:03.952970977 22988 0x7f4a1c579190 LOG basesink gstbasesink.c:2044:gst_base_sink_get_sync_times:<alsasink1> buffer skipped, not in segment 0:00:03.952976721 22988 0x7f4a1c579190 DEBUG basesink gstbasesink.c:2590:gst_base_sink_do_sync:<alsasink1> non syncable object 0x1bb2b20
Review of attachment 325516 [details] [review]: ::: gst/gstsegment.c @@ +887,3 @@ * we're outside of the segment */ + if (G_UNLIKELY (segment->stop != -1 && start != -1 && (start > segment->stop + || (segment->start != segment->stop && start == segment->stop)))) Can you provide a unit test for this too?
Created attachment 326089 [details] [review] segment: Modifiy inside segment condition
(In reply to Sebastian Dröge (slomo) from comment #4) > Review of attachment 325516 [details] [review] [review]: > > ::: gst/gstsegment.c > @@ +887,3 @@ > * we're outside of the segment */ > + if (G_UNLIKELY (segment->stop != -1 && start != -1 && (start > > segment->stop > + || (segment->start != segment->stop && start == > segment->stop)))) > > Can you provide a unit test for this too? Update test code. "segment.start == segment.stop" is very rare case, but it actually happen. Both following cases are inside of segment in my opinion. Case1: segment_start == segment_stop == start Case2: segment_start == segment_stop == stop
Review of attachment 326089 [details] [review]: ::: tests/check/gst/gstsegment.c @@ +358,3 @@ + /* touching boundary point. it's inside because stop at segment start */ + res = gst_segment_clip (&segment, GST_FORMAT_BYTES, + 100, 200, &cstart, &cstop); stop is exclusive, so this should return FALSE @@ +388,3 @@ + + /* start on boundary point */ + res = gst_segment_clip (&segment, GST_FORMAT_BYTES, 200, -1, &cstart, &cstop); Maybe also some with stop!=-1 here?
(In reply to Sebastian Dröge (slomo) from comment #7) > Review of attachment 326089 [details] [review] [review]: > > ::: tests/check/gst/gstsegment.c > @@ +358,3 @@ > + /* touching boundary point. it's inside because stop at segment start */ > + res = gst_segment_clip (&segment, GST_FORMAT_BYTES, > + 100, 200, &cstart, &cstop); > > stop is exclusive, so this should return FALSE One question. Although stop == segment.stop, it's FALSE because stop is exclusive. right? > > @@ +388,3 @@ > + > + /* start on boundary point */ > + res = gst_segment_clip (&segment, GST_FORMAT_BYTES, 200, -1, &cstart, > &cstop); > > Maybe also some with stop!=-1 here?
Created attachment 326106 [details] [review] segment: Modifiy inside segment condition
(In reply to Sebastian Dröge (slomo) from comment #7) > Review of attachment 326089 [details] [review] [review]: > > ::: tests/check/gst/gstsegment.c > @@ +358,3 @@ > + /* touching boundary point. it's inside because stop at segment start */ > + res = gst_segment_clip (&segment, GST_FORMAT_BYTES, > + 100, 200, &cstart, &cstop); > > stop is exclusive, so this should return FALSE Code is changed that return FALSE > > @@ +388,3 @@ > + > + /* start on boundary point */ > + res = gst_segment_clip (&segment, GST_FORMAT_BYTES, 200, -1, &cstart, > &cstop); > > Maybe also some with stop!=-1 here? Add test case with stop != -1 + /* touching boundary point. it's inside because start at segment start */ + res = gst_segment_clip (&segment, GST_FORMAT_BYTES, + 200, 300, &cstart, &cstop); + fail_unless (res == TRUE); + fail_unless (cstart == 200); + fail_unless (cstop == 200);
Created attachment 326108 [details] [review] segment: Modifiy inside segment condition Removing unnecessary white space... sorry
Patch was updated.
commit 15f2898e872d3b7ce74ea5fbd35fac9f1f47b4a5 Author: Seungha Yang <sh.yang@lge.com> Date: Fri Apr 15 20:54:42 2016 +0900 segment: Modifiy inside segment condition There is a special case that segment_start == segment_stop == start. It's inside of segment https://bugzilla.gnome.org/show_bug.cgi?id=764707