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 776303 - vaapisink: race condition during caps negotiation with multiple src elements
vaapisink: race condition during caps negotiation with multiple src elements
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal normal
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 774241
 
 
Reported: 2016-12-20 06:44 UTC by Hyunjun Ko
Modified: 2016-12-21 10:32 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
vaapisink: ensure raw_caps at start (1.06 KB, patch)
2016-12-21 05:30 UTC, Hyunjun Ko 		none Details | Review
vaapisink: ensures raw_caps at start (1.23 KB, patch)
2016-12-21 08:52 UTC, Hyunjun Ko 		committed Details | Review

Description Hyunjun Ko 2016-12-20 06:44:51 UTC 
1. Reproduce step:
# gst-validate-launcher -t validate.file.compositor.simple.seek_backward.bgra

2. Test pipeline
gst-launch-1.0 compositor name=_mixer !  deinterlace ! videoconvert ! autovideosink videotestsrc ! video/x-raw, framerate=\(fraction\)10/1, width=100, height=100 ! _mixer. videotestsrc ! video/x-raw, framerate=\(fraction\)5/1, width=320, height=240 ! _mixer.

3. Result
Sometimes negotiation fails, sometimes crash happens.

4. Cause
During get_caps, vaapisink tries to make possible raw caps based on VAquery via gst_vaapi_plugin_base_get_allowed_raw_caps.
At this moment, race condition causes crash or failure of negotiation.
Comment 1 Hyunjun Ko 2016-12-21 05:30:35 UTC 
Created attachment 342298 [details] [review]
vaapisink: ensure raw_caps at start

Calls gst_vaapi_plugin_base_get_allowed_raw_caps at start so that it avoid
race condition at get_caps, especially with multiple src elements.
Comment 2 Víctor Manuel Jáquez Leal 2016-12-21 08:24:30 UTC 
can you post the crash backtrace?
Comment 3 Hyunjun Ko 2016-12-21 08:30:25 UTC 



+
Trace
    237006






    

  • 
#0
__GI_raise

    
                at ../sysdeps/unix/sysv/linux/raise.c
                  line
                  54

    

    • 

  • 
#1
__GI_abort

    
                at abort.c
                  line
                  89

    

    • 

  • 
#2
__libc_message

    
                at ../sysdeps/posix/libc_fatal.c
                  line
                  175

    

    • 

  • 
#3
malloc_printerr

    • 

  • 
#4
_int_free

    
                at malloc.c
                  line
                  3865

    

    • 

  • 
#5
_int_realloc

    
                at malloc.c
                  line
                  4356

    

    • 

  • 
#6
__GI___libc_realloc

    
                at malloc.c
                  line
                  3043

    

    • 

  • 
#7
g_realloc

    
                from
                /lib/x86_64-linux-gnu/libglib-2.0.so.0

    

    • 

  • 
#8
??

    
                from
                /lib/x86_64-linux-gnu/libglib-2.0.so.0

    

    • 

  • 
#9
g_array_append_vals

    
                from
                /lib/x86_64-linux-gnu/libglib-2.0.so.0

    

    • 

  • 
#10
append_format

    
                at gstvaapidisplay.c
                  line
                  238

    

    • 

  • 
#11
append_formats

    
                at gstvaapidisplay.c
                  line
                  261

    

    • 

  • 
#12
ensure_image_formats

    
                at gstvaapidisplay.c
                  line
                  730

    

    • 

  • 
#13
gst_vaapi_display_get_image_formats

    
                at gstvaapidisplay.c
                  line
                  1561

    

    • 

  • 
#14
ensure_allowed_raw_caps

    
                at gstvaapipluginbase.c
                  line
                  1136

    

    • 

  • 
#15
gst_vaapi_plugin_base_get_allowed_raw_caps

    
                at gstvaapipluginbase.c
                  line
                  1195

    

    • 

  • 
#16
gst_vaapisink_get_caps_impl

    
                at gstvaapisink.c
                  line
                  1250

    

    • 

  • 
#17
gst_vaapisink_get_caps

    
                at gstvaapisink.c
                  line
                  1270

    

    • 

  • 
#18
gst_base_sink_query_caps

    
                at gstbasesink.c
                  line
                  572

    

    • 

  • 
#19
gst_base_sink_default_query

    
                at gstbasesink.c
                  line
                  4979

    

    • 

  • 
#20
gst_validate_pad_monitor_query_func

    
                at gst-validate-pad-monitor.c
                  line
                  2228

    

    • 

  • 
#21
gst_pad_query

    
                at gstpad.c
                  line
                  3949

    

    • 

  • 
#22
gst_pad_peer_query

    
                at gstpad.c
                  line
                  4081

    

    • 

  • 
#23
query_caps_func

    
                at gstutils.c
                  line
                  2621

    

    • 

  • 
#24
gst_pad_forward

    
                at gstpad.c
                  line
                  2945

    

    • 

  • 
#25
gst_pad_proxy_query_caps

    
                at gstutils.c
                  line
                  2671

    

    • 

  • 
#26
gst_pad_query_caps_default

    
                at gstpad.c
                  line
                  3124

    

    • 

  • 
#27
gst_pad_query_default

    
                at gstpad.c
                  line
                  3356

    

    • 

  • 
#28
gst_pad_query

    
                at gstpad.c
                  line
                  3949

    

    • 

  • 
#29
gst_pad_peer_query

    
                at gstpad.c
                  line
                  4081

    

    • 

  • 
#30
gst_pad_peer_query_caps

    
                at gstutils.c
                  line
                  2951

    

    • 

  • 
#31
gst_base_transform_query_caps

    
                at gstbasetransform.c
                  line
                  740

    

    • 

  • 
#32
gst_base_transform_default_query

    
                at gstbasetransform.c
                  line
                  1603

    

    • 

  • 
#33
gst_validate_pad_monitor_query_func

    
                at gst-validate-pad-monitor.c
                  line
                  2228

    

    • 

  • 
#34
gst_pad_query

    
                at gstpad.c
                  line
                  3949

    

    • 

  • 
#35
gst_pad_peer_query

    
                at gstpad.c
                  line
                  4081

    

    • 

  • 
#36
gst_pad_peer_query_caps

    
                at gstutils.c
                  line
                  2951

    

    • 

  • 
#37
gst_deinterlace_getcaps

    
                at gstdeinterlace.c
                  line
                  2315

    

    • 

  • 
#38
gst_deinterlace_sink_query

    
                at gstdeinterlace.c
                  line
                  2965

    

    • 

  • 
#39
gst_validate_pad_monitor_query_func

    
                at gst-validate-pad-monitor.c
                  line
                  2228

    

    • 

  • 
#40
gst_pad_query

    
                at gstpad.c
                  line
                  3949

    

    • 

  • 
#41
gst_pad_peer_query

    
                at gstpad.c
                  line
                  4081

    

    • 

  • 
#42
gst_pad_peer_query_caps

    
                at gstutils.c
                  line
                  2951

    

    • 

  • 
#43
gst_video_aggregator_pad_sink_getcaps

    
                at gstvideoaggregator.c
                  line
                  963

    

    • 

  • 
#44
gst_video_aggregator_sink_query

    
                at gstvideoaggregator.c
                  line
                  2031

    

    • 

  • 
#45
_sink_query

    
                at compositor.c
                  line
                  1096

    

    • 

  • 
#46
gst_validate_pad_monitor_query_func

    
                at gst-validate-pad-monitor.c
                  line
                  2228

    

    • 

  • 
#47
gst_pad_query

    
                at gstpad.c
                  line
                  3949

    

    • 

  • 
#48
gst_pad_peer_query

    
                at gstpad.c
                  line
                  4081

    

    • 

  • 
#49
gst_pad_peer_query_caps

    
                at gstutils.c
                  line
                  2951

    

    • 

  • 
#50
gst_base_transform_query_caps

    
                at gstbasetransform.c
                  line
                  740

    

    • 

  • 
#51
gst_base_transform_default_query

    
                at gstbasetransform.c
                  line
                  1603

    

    • 

  • 
#52
gst_validate_pad_monitor_query_func

    
                at gst-validate-pad-monitor.c
                  line
                  2228

    

    • 

  • 
#53
gst_pad_query

    
                at gstpad.c
                  line
                  3949

    

    • 

  • 
#54
gst_pad_peer_query

    
                at gstpad.c
                  line
                  4081

    

    • 

  • 
#55
gst_pad_peer_query_caps

    
                at gstutils.c
                  line
                  2951

    

    • 

  • 
#56
gst_base_src_default_negotiate

    
                at gstbasesrc.c
                  line
                  3211

    

    • 

  • 
#57
gst_base_src_negotiate

    
                at gstbasesrc.c
                  line
                  3275

    

    • 

  • 
#58
gst_base_src_loop

    
                at gstbasesrc.c
                  line
                  2703

    

    • 

  • 
#59
gst_task_func

    
                at gsttask.c
                  line
                  334

    

    • 

  • 
#60
??

    
                from
                /lib/x86_64-linux-gnu/libglib-2.0.so.0

    

    • 

  • 
#61
??

    
                from
                /lib/x86_64-linux-gnu/libglib-2.0.so.0

    

    • 

  • 
#62
start_thread

    
                at pthread_create.c
                  line
                  333

    

    • 

  • 
#63
clone

    
                at ../sysdeps/unix/sysv/linux/x86_64/clone.S
                  line
                  109

    

    • 













Comment 4


Víctor Manuel Jáquez Leal





          2016-12-21 08:38:22 UTC
        



Ok. Yes, the same race condition as in encoders.






Comment 5


Víctor Manuel Jáquez Leal





          2016-12-21 08:40:35 UTC
        



Review of attachment 342298 [details] [review]:

::: gst/vaapi/gstvaapisink.c
@@ +1221,1 @@
 }

I would change the code style to follow what is used in the project:

if (!gst_vaapi_sink_ensure_display (sink))
  return FALSE;

/* explain here why we are doing this */
if (!gst_vaapi_plugin_base_allowed_raw_caps (GST...))
  return FALSE;

return TRUE;






Comment 6


Hyunjun Ko





          2016-12-21 08:52:24 UTC
        



Created attachment 342311 [details] [review]
vaapisink: ensures raw_caps at start

Calls gst_vaapi_plugin_base_get_allowed_raw_caps at start so that it avoid
race condition at get_caps, especially with multiple src elements.






Comment 7


Víctor Manuel Jáquez Leal





          2016-12-21 10:32:36 UTC
        



committed with minor modifications