GNOME Bugzilla – Bug 786173
vaapi h264 decoder corrupted frame when there is resolution change
Last modified: 2017-09-28 13:25:37 UTC
Created attachment 357440 [details] Test application I'm writing a test application that encode and decode an H.264 stream. The two pipeline (encoder pipeline and decoder pipeline) are connected via an udpsink and udpsrc on the loopback. The problem is when the encoder changes resolution, the output on the decoder is corrupted. I tested with VLC and it is able to show correctly the stream. From logs on the decoder side, I can see that the h264parse send the new caps to vaapidecodebin but the vaapidecode negotiate something like sink pad new resolution src pad old resolution. In attachment my test source code.
Please make it using byte-stream instead of avc when you want to change configuration in the middle of the stream. In your case, put capsfilter "video/x-h264,stream-format=byte-stream" before h264parse element (after depay element) in your decoding pipeline. I confirmed that your example is working after modification above.
Thanks, It works. - Tested on my Kady lake GST 1.13.0 (libva 1.7.1) and it works without any issue. - Tested on Braswell GST 1.12.2 (libva 1.8.1)and it works but after awhile it core dump. On the 1.12.2 it core dump when switching to 1920x1080. Please note that it starts at 1920x1080. Do you know if from 1.12.2 and current master it was fixed something for buffer size like 1080p?
(In reply to Matteo Valdina from comment #2) > - Tested on Braswell GST 1.12.2 (libva 1.8.1)and it works but after awhile > it core dump. > > On the 1.12.2 it core dump when switching to 1920x1080. Please note that it > starts at 1920x1080. Please, upload the core dump to figure out what's the problem.
Created attachment 357564 [details] core dump for 1080p This is the core dump with 1.12.2
(In reply to Matteo Valdina from comment #2) > > On the 1.12.2 it core dump when switching to 1920x1080. Please note that it > starts at 1920x1080. > > Do you know if from 1.12.2 and current master it was fixed something for > buffer size like 1080p? I've seen the segfault problem, seems it should be fixed. I'm looking into this.
This crash is caused by the case of exceeding the buffer size of udpsrc. When 1080p is streamed, slice might be over 65536, which can be failed to parse entire slice IDR. @Matteo, please increase buffer-size of udpsrc. @Victor, do we need to prevent this crash in decoder if something like this happens?
(In reply to Hyunjun Ko from comment #6) > This crash is caused by the case of exceeding the buffer size of udpsrc. > When 1080p is streamed, slice might be over 65536, which can be failed to > parse entire slice IDR. > > @Matteo, please increase buffer-size of udpsrc. > > @Victor, do we need to prevent this crash in decoder if something like this > happens? I'd say we shouldn't crash, but fail elegantly.
Hi @Hyunjun, I did what you asked and it is not crashing on the 1.12.2, but I'm a little confused. From my understanding, the udpsrc buffer-size is the size for the socket. I assume that it is translated to "setsockop(SO_RCV_BUFSIZE. buffer-size value)" or something similar. Is this value used also for another purpose? This value should only affect the buffer size if is too small will lead to a packet loss. vaapih264dec is crashing due to an incomplete slice, like when there is a packet loss? I plan to use this decoder for receiving a stream from a network so a packet loss could happen for multiple reasons.
Hi, I noticed that after awhile that my resolution change test will hang. Checking dmesg logs shown these lines: [ 8116.414921] [drm] stuck on bsd ring [ 8116.424181] [drm] GPU HANG: ecode 8:1:0xcab2c083, in camera-source-0 [24680], reason: Ring hung, action: reset [ 8116.424498] [drm:i915_set_reset_status [i915]] *ERROR* gpu hanging too fast, banning! [ 8116.426752] drm/i915: Resetting chip after gpu hang Should I report a bug to libva/intel-libva-driver?
@Matteo, usually, when a backtrace is requested, normally you just copy&paste the output of the bt command in gdb. There's no need of uploading the dump core, since it's big and almost useless, because you need the original setup to do a complete analysis of the memory. https://wiki.debian.org/HowToGetABacktrace
(In reply to Matteo Valdina from comment #8) > Hi @Hyunjun, > I did what you asked and it is not crashing on the 1.12.2, but I'm a little > confused. > > From my understanding, the udpsrc buffer-size is the size for the socket. I > assume that it is translated to "setsockop(SO_RCV_BUFSIZE. buffer-size > value)" or something similar. > > Is this value used also for another purpose? > This value should only affect the buffer size if is too small will lead to a > packet loss. > vaapih264dec is crashing due to an incomplete slice, like when there is a > packet loss? > > I plan to use this decoder for receiving a stream from a network so a packet > loss could happen for multiple reasons. That might a bug in udpsrc fixed in master, and that needs to be backported to 1.12
(In reply to Matteo Valdina from comment #8) > Hi @Hyunjun, > I did what you asked and it is not crashing on the 1.12.2, but I'm a little > confused. > > From my understanding, the udpsrc buffer-size is the size for the socket. I > assume that it is translated to "setsockop(SO_RCV_BUFSIZE. buffer-size > value)" or something similar. > > Is this value used also for another purpose? > This value should only affect the buffer size if is too small will lead to a > packet loss. > vaapih264dec is crashing due to an incomplete slice, like when there is a > packet loss? > > I plan to use this decoder for receiving a stream from a network so a packet > loss could happen for multiple reasons. Crash should be fixed as Victor said. You can report if this kind of issue haapens. We're also trying to deal with streaming cases. eg. #bug 777506
Created attachment 357685 [details] [review] libs: decoder: h264: flush dpb if IDR is missing If configuration is changed during decoding, decoder requires IDR to reset some state of decoder. But if it fails to get IDR, it could be crashed due to trying to overwrite dpb list. This might happen in case of streaming.
I got bitten by the GPU hang using gstreamer 1.12. I suspect a lot with that evil thread which changes the resolution. I have rewritten the test app using a GSource for the change resolution (since you're using the GMainLoop). With it, there's no GPU hang as far as I have seen.
Created attachment 358262 [details] rewritten test app (in C) This test app is written in C but that shouldn't be a problem or impose a big difference.
with 1.12 * we need to backport bug 754885 (and perhaps others) * the pipeline blocks a while when changing to certain resolutions with either master and 1.12 the corrupted frames remains when the resolution changes, except when it backs to the original resolution.
Created attachment 358392 [details] [review] libs: decoder: h264: improve code-style
Created attachment 358393 [details] [review] vaapidecode: renegotiate always when sink caps change
Created attachment 358394 [details] [review] libs: decoder: at update_caps() decode codec_data When updating the caps in decoder, if the caps has codec_data (avC format), it has to be parsed to update the state of the decoder.
you can patch your test to use stream-format=byte-stream before decoder, then the frames will have the information to decode them correctly (if they are supported by the level). In the case of avc stream-format (the default), there's a bug when resetting the internal decoder. The posted patches aims to solve it. This bug is a regression of bug 781142.
Also, remove the tee in the server side. Until we fix bug 785054, tee might lead to problems.
Comment on attachment 358392 [details] [review] libs: decoder: h264: improve code-style commit d9c88f47
Thanks, I'll test your version.
Review of attachment 358393 [details] [review]: the renegotiations shall be done only if the parser didn't fail after reconfiguring
Comment on attachment 358394 [details] [review] libs: decoder: at update_caps() decode codec_data Attachment 358394 [details] pushed as 2eb2b26 - libs: decoder: at update_caps() decode codec_data
Review of attachment 357685 [details] [review]: I don't understand under what circumstances this dpb overwrite occur. I reproduced with the provided test app, but I'm not sure if the app does the correct thing.
Comment on attachment 357685 [details] [review] libs: decoder: h264: flush dpb if IDR is missing this should not happen, the decoder should flush, purge or drain the pending frames before changing caps
Created attachment 360367 [details] [review] vaapidecode: call drain() when setting new format
Created attachment 360368 [details] [review] vaapiencode: flush pending frames before set format
Created attachment 360414 [details] [review] vaapidecode: drain pending frames before set format Drain pending frames, if any, in the internal decoder before setting the new negotiated format.
Created attachment 360415 [details] [review] vaapiencode: flush pending frames before set format Flush pending frames, if any, in the internal encorder, before setting the new negotiated format.
Attachment 360414 [details] pushed as a4daa2a - vaapidecode: drain pending frames before set format Attachment 360415 [details] pushed as 9f4a576 - vaapiencode: flush pending frames before set format
branch 1.12 * e5ef90f9 vaapiencode: flush pending frames before set format * 6b98d6a0 vaapidecode: drain pending frames before set format * a4c60fb2 libs: decoder: h264/h265: decode codec data only if opened * 9cf26c81 libs: decoder: at update_caps() decode codec_data