After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 783726 - vaapi: failure of some tcs in gst-validate-launcher
vaapi: failure of some tcs in gst-validate-launcher
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.12.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-13 06:27 UTC by Hyunjun Ko
Modified: 2017-06-15 12:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libs: decoder: h264: try to fill gaps even if no previous reference frame (1.56 KB, patch)
2017-06-13 07:08 UTC, Hyunjun Ko
none Details | Review
libs: decoder: h264: initialize active_sps/pps in reset (1.45 KB, patch)
2017-06-14 12:58 UTC, Hyunjun Ko
committed Details | Review

Description Hyunjun Ko 2017-06-13 06:27:02 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:
  • #0 __GI_raise
    at ../sysdeps/unix/sysv/linux/raise.c line 54
  • #1 __GI_abort
    at abort.c line 89
  • #2 g_assertion_message
    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
  • #3 g_assertion_message_expr
    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
  • #4 fill_picture_gaps
    at gstvaapidecoder_h264.c line 3187
  • #5 init_picture
    at gstvaapidecoder_h264.c line 3337
  • #6 decode_picture
    at gstvaapidecoder_h264.c line 4035
  • #7 gst_vaapi_decoder_h264_start_frame
    at gstvaapidecoder_h264.c line 4658
  • #8 do_decode_1
    at gstvaapidecoder.c line 239
  • #9 do_decode
    at gstvaapidecoder.c line 277
  • #10 gst_vaapi_decoder_decode
    at gstvaapidecoder.c line 1039
  • #11 gst_vaapidecode_handle_frame
    at gstvaapidecode.c line 674
  • #12 gst_video_decoder_decode_frame
    at gstvideodecoder.c line 3410
  • #13 gst_video_decoder_have_frame
    at gstvideodecoder.c line 3342
  • #14 gst_vaapidecode_parse_frame
    at gstvaapidecode.c line 1103
  • #15 gst_vaapidecode_parse
    at gstvaapidecode.c line 1134
  • #16 gst_video_decoder_parse_available
    at gstvideodecoder.c line 877
  • #17 gst_video_decoder_chain_forward
    at gstvideodecoder.c line 2153
  • #18 gst_video_decoder_chain
    at gstvideodecoder.c line 2451
  • #19 gst_validate_pad_monitor_chain_func
    at gst-validate-pad-monitor.c line 2140
  • #20 gst_pad_chain_data_unchecked
    at gstpad.c line 4205
  • #21 gst_pad_push_data
    at gstpad.c line 4457
  • #22 gst_pad_push
    at gstpad.c line 4576
  • #23 gst_proxy_pad_chain_default
    at gstghostpad.c line 127
  • #24 gst_pad_chain_data_unchecked
    at gstpad.c line 4205
  • #25 gst_pad_push_data
    at gstpad.c line 4457
  • #26 gst_pad_push
    at gstpad.c line 4576
  • #27 gst_base_transform_chain
    at gstbasetransform.c line 2312
  • #28 gst_validate_pad_monitor_chain_func
    at gst-validate-pad-monitor.c line 2140
  • #29 gst_pad_chain_data_unchecked
    at gstpad.c line 4205
  • #30 gst_pad_push_data
    at gstpad.c line 4457
  • #31 gst_pad_push
    at gstpad.c line 4576
  • #32 gst_base_parse_push_frame
    at gstbaseparse.c line 2520
  • #33 gst_base_parse_handle_and_push_frame
    at gstbaseparse.c line 2337
  • #34 gst_base_parse_finish_frame
    at gstbaseparse.c line 2678
  • #35 gst_h264_parse_handle_frame_packetized
    at gsth264parse.c line 1023
  • #36 gst_h264_parse_handle_frame
    at gsth264parse.c line 1076
  • #37 gst_base_parse_handle_buffer
    at gstbaseparse.c line 2145
  • #38 gst_base_parse_chain
    at gstbaseparse.c line 3227
  • #39 gst_validate_pad_monitor_chain_func
    at gst-validate-pad-monitor.c line 2140
  • #40 gst_pad_chain_data_unchecked
    at gstpad.c line 4205
  • #41 gst_pad_push_data
    at gstpad.c line 4457
  • #42 gst_pad_push
    at gstpad.c line 4576
  • #43 gst_single_queue_push_one
    at gstmultiqueue.c line 1608
  • #44 gst_multi_queue_loop
    at gstmultiqueue.c line 1920
  • #45 gst_task_func
    at gsttask.c line 335
  • #46 ??
    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
  • #47 ??
    from /lib/x86_64-linux-gnu/libglib-2.0.so.0
  • #48 start_thread
    at pthread_create.c line 333
  • #49 clone
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S line 109

Comment 1 Hyunjun Ko 2017-06-13 06:44:32 UTC
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.
Comment 2 Hyunjun Ko 2017-06-13 07:08:53 UTC
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.
Comment 3 Víctor Manuel Jáquez Leal 2017-06-13 15:32:39 UTC
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.
Comment 4 Hyunjun Ko 2017-06-14 12:58:01 UTC
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.
Comment 5 Víctor Manuel Jáquez Leal 2017-06-14 16:41:39 UTC
Review of attachment 353741 [details] [review]:

looks much more better, indeed!
Comment 6 Víctor Manuel Jáquez Leal 2017-06-15 11:59:21 UTC
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! :)
Comment 7 Víctor Manuel Jáquez Leal 2017-06-15 12:00:32 UTC
Attachment 353741 [details] pushed as 9397cd7 - libs: decoder: h264: initialize active_sps/pps in reset
Comment 8 Víctor Manuel Jáquez Leal 2017-06-15 12:20:21 UTC
In branch 1.12:

* 31e810d4 libs: decoder: h264: initialize active_sps/pps in reset