GNOME Bugzilla – Bug 721835
videodecoder: do not drop events when releasing frames
Last modified: 2014-01-16 05:44:59 UTC
The client pipeline is something like: rtspsrc location=... latency=200 drop-on-latency=true ! decodebin ! videoconvert ! ximagesink Sometimes I am getting this repeatedly: (lt-basic-test:35942): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed It seems that the video decoder starts pushing frame before we get a segment. See the stack trace and the prints of the variables at the frame in question. Program received signal SIGTRAP, Trace/breakpoint trap.
+ Trace 233011
Thread 140736875980544 (LWP 35984)
(gdb) p *segment $2 = {flags = GST_SEGMENT_FLAG_NONE, rate = 1, applied_rate = 1, format = GST_FORMAT_UNDEFINED, base = 0, offset = 0, start = 0, stop = 18446744073709551615, time = 0, position = 0, duration = 18446744073709551615, _gst_reserved = {0x0, 0x0, 0x0, 0x0}}
Created attachment 265798 [details] with GST_DEBUG=4
It always happens when this error shows up: 0:00:00.410876184 36813 0x7f18fc001e30 INFO GST_STATES gstelement.c:2233:_priv_gst_element_state_changed:<decodebin0> notifying about state-changed READY to PAUSED (VOID_PENDING pending) 0:00:00.411473829 36813 0x7f18fc001e30 INFO GST_EVENT gstevent.c:628:gst_event_new_caps: creating caps event video/x-raw, width=(int)1920, height=(int)1080, pixel-aspect-ratio=(fraction)1 0:00:00.420487459 36813 0x7f18fc001e30 ERROR libav :0:: Missing reference picture 0:00:00.420551846 36813 0x7f18fc001e30 ERROR libav :0:: decode_slice_header error 0:00:00.420837758 36813 0x7f18fc001e30 INFO libav :0:: concealing 8160 DC, 8160 AC, 8160 MV errors 0:00:00.476357212 36813 0x7f18fc001e30 INFO basetransform gstbasetransform.c:1335:gst_base_transform_setcaps:<capsfilter0> reuse caps The server (gst-rtsp-server based) has a dynamic payloader. May be that's something to look at.
Now that I look at the description, it seems I missed to paste a bunch of text. Just in case it's not obvious from the other comments and to keep it short: the playback is streaming H.264 video (generated by x264enc) from a gst-rtsp-server based server. The server uses dynamic payloading.
Created attachment 265839 [details] videodecoder logs for the same problem I can experiment the same behavior while seeking using GST_FORMAT_TIME from my source.
It seems the gst_segment_clip fails because the videodecoder's output_segment format is GST_FORMAT_UNDEFINED
(In reply to comment #4) > Created an attachment (id=265839) [details] > videodecoder logs for the same problem > > I can experiment the same behavior while seeking using GST_FORMAT_TIME from my > source. Your bug seems exactly bug 681634. Same function is called and same error occurs, but it seems to me different ways to trigger it.
Created attachment 265872 [details] [review] set segments to GST_FORMAT_TIME when resetting This fixes it for me.
I can reproduce this too, though resetting the segment to TIME format only hides the issue. We should have received the segment before we received any new encoded buffers, which is way before we reach the point we need to push a buffer.
This also seems to be related to https://bugzilla.gnome.org/show_bug.cgi?id=721666 I'm working on a patch for it, should be available soon.
Created attachment 265885 [details] [review] process segment events before chaining A similar patch for bug 721666 which also seems to work https://bugzilla.gnome.org/attachment.cgi?id=265712&action=edit No UT or comments added yet.
(In reply to comment #10) > Created an attachment (id=265885) [details] [review] > process segment events before chaining > > A similar patch for bug 721666 which also seems to work > > https://bugzilla.gnome.org/attachment.cgi?id=265712&action=edit > Similar to this one, sorry. https://bugzilla.gnome.org/attachment.cgi?id=265711&action=edit The difference is that the processing of the events is done in the chain function.
Comment on attachment 265885 [details] [review] process segment events before chaining Doing the processing in the chain function is not ideal as we should only apply the output segment on the output side of the decoder. You could now apply the new output segment already while the decoder is still working on the previous segment and outputting buffers for that. Does Thiago's patch fix it for your case too?
Can you try the latest patch at https://bugzilla.gnome.org/show_bug.cgi?id=721666 please?
(In reply to comment #13) > Can you try the latest patch at > https://bugzilla.gnome.org/show_bug.cgi?id=721666 please? Is there a way to apply the patch to 1.2.2 version of gstreamer? It seems I can't play my media with HEAD build...
(In reply to comment #14) > (In reply to comment #13) > > Can you try the latest patch at > > https://bugzilla.gnome.org/show_bug.cgi?id=721666 please? > > Is there a way to apply the patch to 1.2.2 version of gstreamer? > It seems I can't play my media with HEAD build... The patch for videodecoder applies cleanly on 1.2.2, the other one is just an unit test, so you don't need it.
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > Can you try the latest patch at > > > https://bugzilla.gnome.org/show_bug.cgi?id=721666 please? > > > > Is there a way to apply the patch to 1.2.2 version of gstreamer? > > It seems I can't play my media with HEAD build... > > The patch for videodecoder applies cleanly on 1.2.2, the other one is just an > unit test, so you don't need it. How should I apply it? I tried downloading and aply it with patch and git apply with no chance? patch give this error : |diff --git a/gst-libs/gst/video/gstvideodecoder.c b/gst-libs/gst/video/gstvideodecoder.c |index de39b8f..5d41534 100644 |--- a/gst-libs/gst/video/gstvideodecoder.c |+++ b/gst-libs/gst/video/gstvideodecoder.c -------------------------- File to patch: gst-libs/gst/video/gstvideodecoder.c patching file gst-libs/gst/video/gstvideodecoder.c Hunk #1 succeeded at 1868 with fuzz 2 (offset -70 lines). patch unexpectedly ends in middle of line and git : error: patch failed: gst-libs/gst/video/gstvideodecoder.c:1938 error: gst-libs/gst/video/gstvideodecoder.c: patch does not apply
Did the patch download successfully? The 'patch unexpectedly ends in middle of line' error is very weird. I applied it here using 'git am file.patch' and it went cleanly.
(In reply to comment #17) > Did the patch download successfully? The 'patch unexpectedly ends in middle of > line' error is very weird. I applied it here using 'git am file.patch' and it > went cleanly. I used 'git apply'; Tried 'git am' but doesn't seem to apply. i think it might be due to the fact I juste downloaded the source for the tar.gz, not from git.
(In reply to comment #18) > (In reply to comment #17) > > Did the patch download successfully? The 'patch unexpectedly ends in middle of > > line' error is very weird. I applied it here using 'git am file.patch' and it > > went cleanly. > > I used 'git apply'; > Tried 'git am' but doesn't seem to apply. > i think it might be due to the fact I juste downloaded the source for the > tar.gz, not from git. In this case try: patch -p1 < thepatchfile.patch (from the extracted source directory)
my apology. I used 'save' from the browser to I had some piece of html in the patch file. So I could apply the patch, but I still have the message : (GStreamerPlayer3:23837): GStreamer-CRITICAL **: gst_segment_clip: assertion `segment->format == format' failed Maybe I can provide some more logs?
Yes, please. Is there an easy way to reproduce the issue?
(In reply to comment #13) > Can you try the latest patch at > https://bugzilla.gnome.org/show_bug.cgi?id=721666 please? I still can reproduce it (see below). I think the patch only solves the problem for reverse playback. In this bug there's no reverse playback involved (if I understand what reverse playback means, rate < 0?). My patch in comment 10 I think solved the issue for both cases, but as Sebastian pointed out in comment 12 that patch is not correct. ----- :00:00.393383184 12882 0x7f0394001e30 INFO GST_EVENT gstevent.c:628:gst_event_new_caps: creating caps event video/x-raw, width=(int)1920, height=(int)1080, pixel-aspect-ratio=(fraction)1/1, interlace-mode=(string)progressive, framerate=(fra 0:00:00.400924137 12882 0x7f0394001e30 ERROR libav :0:: Missing reference picture 0:00:00.400963067 12882 0x7f0394001e30 ERROR libav :0:: decode_slice_header error 0:00:00.401251100 12882 0x7f0394001e30 INFO libav :0:: concealing 8160 DC, 8160 AC, 8160 MV errors 0:00:00.457880494 12882 0x7f0394001e30 INFO basetransform gstbasetransform.c:1335:gst_base_transform_setcaps:<capsfilter0> reuse caps (gst-launch-1.0:12882): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed (gst-launch-1.0:12882): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed (gst-launch-1.0:12882): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed (gst-launch-1.0:12882): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed (gst-launch-1.0:12882): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed (gst-launch-1.0:12882): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed (gst-launch-1.0:12882): GStreamer-CRITICAL **: gst_segment_clip: assertion 'segment->format == format' failed
Created attachment 265987 [details] [review] videodecoder: do not lose events when dropping frames Finally understood what is possibly happening here. 1) after the seek, the videodecoder is flushed 2) videodecoder gets a new segment 3) videodecoder receives its first buffer and attaches the segment event to it 4) avdec_h264 tries to decode this buffer, but it is not a keyframe and it can't be decoded, so it skips it 5) videodecoder receives more buffers and still can't decode any of those buffers 6) videodecoder finally gets a keyframe and is now able to decode this buffer, after decoding it, avdec_h264 goes over the list of pending frames and drops those that won't be useful anymore. The segment event is now lost 7) assertion! Let's hope this is the case. Could you please test this patch?
Created attachment 265988 [details] [review] tests: videodecoder: check that segment events are not dropped Adds a test that simulates a scenario where the first buffers after a segment can't be decoded and the decoder asks for those frames to be released. The videodecoder base class should make sure that the events attached to those first buffers are pushed even if the buffers aren't going to be.
(In reply to comment #23) > Created an attachment (id=265987) [details] [review] > videodecoder: do not lose events when dropping frames > > Finally understood what is possibly happening here. > > 1) after the seek, the videodecoder is flushed > 2) videodecoder gets a new segment > 3) videodecoder receives its first buffer and attaches the segment event to it > 4) avdec_h264 tries to decode this buffer, but it is not a keyframe and it > can't > be decoded, so it skips it > 5) videodecoder receives more buffers and still can't decode any of those > buffers > 6) videodecoder finally gets a keyframe and is now able to decode this buffer, > after decoding it, avdec_h264 goes over the list of pending frames and drops > those that won't be useful anymore. The segment event is now lost > 7) assertion! > > Let's hope this is the case. Could you please test this patch? Yes, this all sounds great. As I mentioned in comment 2 it only happens when first frames can't be decoded. I see if I can try it over the weekend but I don't promise anything as my failing test case is at work. Thanks for the work!
Review of attachment 265987 [details] [review]: While this generally makes sense, why isn't the pending_events code in gst_video_decoder_prepare_finish_frame() taking care of this already? That's why it is there :)
(In reply to comment #26) > Review of attachment 265987 [details] [review]: > > While this generally makes sense, why isn't the pending_events code in > gst_video_decoder_prepare_finish_frame() taking care of this already? That's > why it is there :) It is because the frame that has the segment event never gets to _prepare_finish_frame, libav decoder will call _release_frame that just discards the frame and everything with it. IIRC if it called _drop_frame it would go into _prepare_finish_frame before dropping, but it has other side effects like updating the position and stuff like that. Anyway I guess it is dangerous that the base class allows those events to be dropped in _release_frame.
Comment on attachment 265987 [details] [review] videodecoder: do not lose events when dropping frames Makes sense, get it in :) Also thanks for creating a test for this too
I am also seeing similar issue. There are two patches given. https://bugzilla.gnome.org/attachment.cgi?id=265987 https://bugzilla.gnome.org/review?bug=721835&attachment=265988 Should I apply both patches.?
the latest patch fixes the issue I had thx!
Eric, 1. You meant this patch https://bugzilla.gnome.org/attachment.cgi?id=265987 ?? 2. Is this patch given to 1.2.1 version?? 3. I see line number mismatch between the given patch and the code downloaded from here http://gstreamer.freedesktop.org/src/gst-plugins-base/gst-plugins-base-1.2.2.tar.xz. Am I missing any other previous patches?? Thank you for your help. Good day.
(In reply to comment #31) > Eric, > 1. You meant this patch https://bugzilla.gnome.org/attachment.cgi?id=265987 ?? Yes, this one > 2. Is this patch given to 1.2.1 version?? I applied it on 1.2.2 > 3. I see line number mismatch between the given patch and the code downloaded > from here > http://gstreamer.freedesktop.org/src/gst-plugins-base/gst-plugins-base-1.2.2.tar.xz. > Am I missing any other previous patches?? I have also applied the patch for reverse playback, but this doesn't do anything as I am playing forward, and thus the code isn't called. > > Thank you for your help. Good day.
Patches pushed to master. commit 561a4fff158c6ab718b574d39bb7e5383129fb23 Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Sat Jan 11 01:14:19 2014 -0300 tests: videodecoder: check that segment events are not dropped Adds a test that simulates a scenario where the first buffers after a segment can't be decoded and the decoder asks for those frames to be released. The videodecoder base class should make sure that the events attached to those first buffers are pushed even if the buffers aren't going to be. https://bugzilla.gnome.org/show_bug.cgi?id=721835 commit 672cda66db65174489e315ad8937f8e0ea7c7d81 Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Sat Jan 11 01:24:44 2014 -0300 videodecoder: do not lose events when dropping frames Events must be persisted after a frame is dropped to avoid losing obligatory information for the stream. https://bugzilla.gnome.org/show_bug.cgi?id=721835
Pushed to 1.2 commit cb49884b6aae52adb08b0349775386e39af37cfe Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Sat Jan 11 01:14:19 2014 -0300 tests: videodecoder: check that segment events are not dropped Adds a test that simulates a scenario where the first buffers after a segment can't be decoded and the decoder asks for those frames to be released. The videodecoder base class should make sure that the events attached to those first buffers are pushed even if the buffers aren't going to be. https://bugzilla.gnome.org/show_bug.cgi?id=721835 commit 2acde77b6c642985862ce735d6865009a01505ba Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Sat Jan 11 01:24:44 2014 -0300 videodecoder: do not lose events when dropping frames Events must be persisted after a frame is dropped to avoid losing obligatory information for the stream. https://bugzilla.gnome.org/show_bug.cgi?id=721835
Just wanted to confirm that this also fixes things for me. Thanks!
(In reply to comment #34) > Pushed to 1.2 > Any reason why all these patches didn't make it to master?
(In reply to comment #36) > (In reply to comment #34) > > Pushed to 1.2 > > > > Any reason why all these patches didn't make it to master? They were pushed both to master and 1.2 branch
(In reply to comment #37) > (In reply to comment #36) > > (In reply to comment #34) > > > Pushed to 1.2 > > > > > > > Any reason why all these patches didn't make it to master? > > They were pushed both to master and 1.2 branch Yes, I see now, sorry about that. I pulled from another master branch that was not updated and I was sure it was the right one... didn't double check or check cgit. Anyway, sorry again.