GNOME Bugzilla – Bug 753090
AVI/JPEG reverse playback: buffer sent before segment event after seek
Last modified: 2018-11-03 15:02:51 UTC
Created attachment 308514 [details] add condition about segment format check videobalance: add condition about segment format check This element transformed buffer timestamp for synchronizing. But it tried to transform timestamp before the new segment comes up. It has to transform after segment values are set. Otherwise, gstreamer occur critical issue by segment translation logic. Issue is reproduced mjpeg codec by running gst-validate-1.0 GST_VALIDATE_SCENARIO=reverse_playback GST_GL_XINITTHREADS=1 playbin uri=file:///home/gst-validate/gst-integration-testsuites/medias/defaults/avi/bowlerhatdancer.sleepytom.SGP.mjpeg.avi audio-sink='fakesink sync=true' video-sink='fakesink sync=true' --set-media-info /home/gst-validate/gst-integration-testsuites/medias/defaults/avi/bowlerhatdancer.sleepytom.SGP.mjpeg.avi.media_info
Created attachment 308516 [details] [review] add condition about segment format check This element transformed buffer timestamp for synchronizing. But it tried to transform timestamp before the new segment comes up. It has to transform after segment values are set. Otherwise, gstreamer occur critical issue by segment translation logic.
Comment on attachment 308516 [details] [review] add condition about segment format check This does not look right to me. When we are processing buffers, we should have received a segment event. It looks like for some reason we have not received a segment event. That would be the fault of some upstream element then.
PS: when trying to come up with a bug title or commit message summary line, try to answer the question "What problem/issue does this fix?". We can look at the patch to see *how* the problem was solved :)
Created attachment 309049 [details] [review] basetransform: check the segment condition basetransform: check the segment condition Problem: In gst-validate test, mjpeg-reverse-playback-test with fakesink is failed because gstreamer occur critical error by segment transformation function. Critical error gst_segment_to_stream_time: assertion 'segment->format == format' failed Caused: Basetransform reset the segment values when it received flush stop event. Before the segment is set, basetransform try to execute transformation segment. Steps to Reproduce: GST_VALIDATE_SCENARIO=reverse_playback GST_GL_XINITTHREADS=1 playbin uri=file:///home/gst-validate/gst-integration-testsuites/medias/defaults/avi/bowlerhatdancer.sleepytom.SGP.mjpeg.avi audio-sink='fakesink sync=true' video-sink='fakesink sync=true' --set-media-info /home/gst-validate/gst-integration-testsuites/medias/defaults/avi/bowlerhatdancer.sleepytom.SGP.mjpeg.avi.media_info Resolution: Basetransform need to check the segment values before it executes transformation segment.
(In reply to Tim-Philipp Müller from comment #2) > Comment on attachment 308516 [details] [review] [review] > add condition about segment format check > > This does not look right to me. When we are processing buffers, we should > have received a segment event. It looks like for some reason we have not > received a segment event. That would be the fault of some upstream element > then. I found the root cause, upload new patch. Please refer to commit message.
My comment still applies: we should have received a segment event again before getting the first buffer after a flush. Is this not happening here?
(In reply to Tim-Philipp Müller from comment #6) > My comment still applies: we should have received a segment event again > before getting the first buffer after a flush. Is this not happening here? I just check it. It is not happening as you expect. The event sequence is: seek(rate=-2) -> flush -> one buffer pusshed(problem occured!!) -> received segment Is the one buffer root caused, because videobalance element received the buffer before getting segment? I have no idea how I can analyze it. :-(
The sequence is not right. What is the previous element upstream? It sounds to me like another element sends things in the wrong sequence.
The previous upstream element is "videoscale". But, this element cannot handle segment event and buffer push. Well, I will also check avi demux and decoder element(jpegdec). Thanks for your help.
Created attachment 310468 [details] [review] videodecoder: check the segment status I found the root cause. The decoder sends invalid buffer to downstream element, even though it doesn't send the segment event. Please refer to patch comment. =========== Usually, if the element finished flushing, then it sent segment event. And then it pushed buffer. If videodecoder has no segment value, it means videodecoder is in flushing status except first playing time. Videodecoder pushed the buffer even though it doesn't have segment value. Therefore, videodecoder have to check the segment status.
You could also check for segment.format==GST_FORMAT_UNDEFINED I guess, instead of adding a new variable. How can the segment be reset and then we push a buffer? The resetting happens in FLUSH_STOP, at which time the stream lock should be taken and no data should be flowing anymore. Is the problem that a buffer arrives *after* FLUSH_STOP but before a new SEGMENT event arrived? If so, the problem is somewhere else. Nothing should ever push buffers before SEGMENT events.
Created attachment 310557 [details] [review] videodecoder: check the segment status It can be checked by the condition of "segment.format==GST_FORMAT_UNDEFINED". I will change it. :) Unfortunately, it can be happened by timing. Because demux pushed the buffer as soon as it finished to send segment event. But at the same time, the decoder is flushing. It means that decoder received the buffer in the middle of handling flush event. Also decoder receive the segment event. Now, in normal case, decoder handle the receive the segment, and then pushed the buffer. But, in abnormal case buffer is handled within a short time between finishing flush event and handling segment event. In all of more than 100 tests, it is happened just one testcase. :-(
Created attachment 310559 [details] [review] videodecoder: check the segment status it just modified email.
Created attachment 310560 [details] [review] videodecoder: check the segment status update.
> Unfortunately, it can be happened by timing. Because demux pushed the buffer > as soon as it finished to send segment event. > But at the same time, the decoder is flushing. It means that decoder > received the buffer in the middle of handling flush event. Also decoder > receive the segment event. This doesn't sound right. The demuxer should send: - FLUSH_START - FLUSH_STOP (serialised) - SEGMENT (serialised) - BUFFER (serialised) In what order does the decoder receive those events on its sink pad? The decoder should never receive the buffer in the middle of handling the flush event, because FLUSH_STOP/SEGMENT/buffers are serialised and can't jump ahead of each other, and should be sent in that exact order. So something still doesn't seem quite right.
(In reply to Tim-Philipp Müller from comment #15) > > Unfortunately, it can be happened by timing. Because demux pushed the buffer > > as soon as it finished to send segment event. > > But at the same time, the decoder is flushing. It means that decoder > > received the buffer in the middle of handling flush event. Also decoder > > receive the segment event. > > This doesn't sound right. > > The demuxer should send: > - FLUSH_START > - FLUSH_STOP (serialised) > - SEGMENT (serialised) > - BUFFER (serialised) > > In what order does the decoder receive those events on its sink pad? > > The decoder should never receive the buffer in the middle of handling the > flush event, because FLUSH_STOP/SEGMENT/buffers are serialised and can't > jump ahead of each other, and should be sent in that exact order. > > So something still doesn't seem quite right. "the middle of handling flush event" means that the decoder is resetting segment values. It means the segment is not setting any values yet. I think that "not setting the segment" is same meaning of "not finishing flush", even though it receive segment event. Because the video sink receive the segment event but it do nothing, so I just think it's caused by "flush event". -> The cause of doing nothing is just my speculation. It is can be misunderstood. :) Anyway, I also curious about it and analyze the log again. The sequence is: [In videodecoder] - flush start. Line:1363 - flush stop. Line:1449 - receive segment. Line:1492 - receive buffer. Line:2492 - setting segment value. Line:2507 The decoder is not setting the segment value as soon as received segment event. I have no idea about this, because the demux and decoder just push and parse the stream. I attached the log also.
Created attachment 310570 [details] The log for checking the status. The log for checking the status.
That sounds like a bug in the video decoder base class then. After receiving the segment and before handling the first buffer, it should have everything set up to actually correlate the buffer to the previous segment event.
-- 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/gst-plugins-good/issues/208.