GNOME Bugzilla – Bug 783726
vaapi: failure of some tcs in gst-validate-launcher
Last modified: 2017-06-15 12:20:21 UTC
I found there are some failures for 3 testcases due to "Application segfaulted" as the following: gst-validate-launcher -t validate.file.playback.scrub_forward_seeking.test5_mkv gst-validate-launcher -t validate.file.playback.seek_backward.test5_mkv gst-validate-launcher -t validate.file.playback.seek_forward.test5_mkv This is introduced by commits on bug 781142. callstack:
+ Trace 237570
Also, you can reproduce by just doing some seek of that media. gst-play-1.0 ///home/zzoon/gst-validate/gst-integration-testsuites/medias/defaults/matroska/test5.mkv This is because there happens the case that no reference frame yet when it tries to decode slice after all status in decoder is refreshed in reset during seek. Normally, after seek it just skip fill_picture_gaps since frame number is followed by previous frame number. But in this case, frame number after seek jumped to higher, which means there's gap between prev frame number and current frame number. Then it starts trying to fill gap of pictures but it failed due to no previous ref frame.
Created attachment 353656 [details] [review] libs: decoder: h264: try to fill gaps even if no previous reference frame Makes it trying to fill gaps even if no previous reference frame instead of assertion. Not sure if this approace is correct. Need to review.
Review of attachment 353656 [details] [review]: ::: gst-libs/gst/vaapi/gstvaapidecoder_h264.c @@ +3187,3 @@ + prev_picture = + prev_frame ? gst_vaapi_picture_ref (prev_frame->buffers[0]) : NULL; + These kind of patches are worrisome, since they, by catching NULLs are just hiding the original problem: why that NULL appears? IMO the regression needs to complete the patch that didn't foresee this situation where there is no previous frame.
Created attachment 353741 [details] [review] libs: decoder: h264: initialize active_sps/pps in reset Since commits in https://bugzilla.gnome.org/show_bug.cgi?id=781142 landed, they introduced regression in seek. Formerly, once seek is done, decoder drops P-frames until I-frame arrives. But since the commits landed, it doesn't drop P-frame and does try to decode it continuously because active_sps is still alive. See ensure_sps function. But there are prev_frames and prev_ref_frames reset already, then it causes assertion. So it's necessary to reset active_sps/pps also in reset method.
Review of attachment 353741 [details] [review]: looks much more better, indeed!
testing, it seems that gst-validate-launcher -t validate.file.playback.scrub_forward_seeking.test5_mkv still fails: Failed 'Application returned 18 (critical errors: [A segment doesn't have the proper time value after an ACCURATE seek])' But it is related with the subtitles in the mkv containter. The problem before was a crash because of gstreamer-vaapi, so it is fixed! :)
Attachment 353741 [details] pushed as 9397cd7 - libs: decoder: h264: initialize active_sps/pps in reset
In branch 1.12: * 31e810d4 libs: decoder: h264: initialize active_sps/pps in reset