va: Fix a latent race condition in vabasedec.

The vabasedec's display and decoder are created/destroyed between
the gst_va_base_dec_open/close pair. All the data and event handling
functions are between this pair and so the accessing to these pointers
are safe. But the query function can be called anytime. So we need to:
1. Make these pointers operation in open/close and query atomic.
2. Hold an extra ref during query function to avoid it destroyed.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1957>
This commit is contained in:
He Junyan 2021-01-15 15:22:07 +08:00
parent 4d07974b10
commit 6b1e1924fd
2 changed files with 58 additions and 19 deletions

View file

@ -36,15 +36,27 @@ gst_va_base_dec_open (GstVideoDecoder * decoder)
{
GstVaBaseDec *base = GST_VA_BASE_DEC (decoder);
GstVaBaseDecClass *klass = GST_VA_BASE_DEC_GET_CLASS (decoder);
gboolean ret = FALSE;
if (!gst_va_ensure_element_data (decoder, klass->render_device_path,
&base->display))
return FALSE;
if (!base->decoder)
base->decoder = gst_va_decoder_new (base->display, klass->codec);
if (!g_atomic_pointer_get (&base->decoder)) {
GstVaDecoder *va_decoder;
return (base->decoder != NULL);
va_decoder = gst_va_decoder_new (base->display, klass->codec);
if (va_decoder)
ret = TRUE;
gst_object_replace ((GstObject **) (&base->decoder),
(GstObject *) va_decoder);
gst_object_unref (va_decoder);
} else {
ret = TRUE;
}
return ret;
}
gboolean
@ -82,9 +94,14 @@ gst_va_base_dec_getcaps (GstVideoDecoder * decoder, GstCaps * filter)
{
GstCaps *caps = NULL, *tmp;
GstVaBaseDec *base = GST_VA_BASE_DEC (decoder);
GstVaDecoder *va_decoder = NULL;
if (base->decoder)
caps = gst_va_decoder_get_sinkpad_caps (base->decoder);
gst_object_replace ((GstObject **) & va_decoder, (GstObject *) base->decoder);
if (va_decoder)
caps = gst_va_decoder_get_sinkpad_caps (va_decoder);
gst_object_unref (va_decoder);
if (caps) {
if (filter) {
@ -108,19 +125,34 @@ gst_va_base_dec_src_query (GstVideoDecoder * decoder, GstQuery * query)
switch (GST_QUERY_TYPE (query)) {
case GST_QUERY_CONTEXT:{
return gst_va_handle_context_query (GST_ELEMENT_CAST (decoder), query,
base->display);
GstVaDisplay *display = NULL;
gst_object_replace ((GstObject **) & display,
(GstObject *) base->display);
ret = gst_va_handle_context_query (GST_ELEMENT_CAST (decoder), query,
display);
gst_object_unref (display);
break;
}
case GST_QUERY_CAPS:{
GstCaps *caps = NULL, *tmp, *filter = NULL;
GstVaDecoder *va_decoder = NULL;
gboolean fixed_caps;
gst_object_replace ((GstObject **) & va_decoder,
(GstObject *) base->decoder);
gst_query_parse_caps (query, &filter);
fixed_caps = GST_PAD_IS_FIXED_CAPS (GST_VIDEO_DECODER_SRC_PAD (decoder));
if (!fixed_caps && base->decoder)
caps = gst_va_decoder_get_srcpad_caps (base->decoder);
if (!fixed_caps && va_decoder)
caps = gst_va_decoder_get_srcpad_caps (va_decoder);
gst_object_unref (va_decoder);
if (caps) {
if (filter) {
tmp =
@ -149,10 +181,18 @@ static gboolean
gst_va_base_dec_sink_query (GstVideoDecoder * decoder, GstQuery * query)
{
GstVaBaseDec *base = GST_VA_BASE_DEC (decoder);
gboolean ret = FALSE;
if (GST_QUERY_TYPE (query) == GST_QUERY_CONTEXT) {
return gst_va_handle_context_query (GST_ELEMENT_CAST (decoder), query,
base->display);
GstVaDisplay *display = NULL;
gst_object_replace ((GstObject **) & display, (GstObject *) base->display);
ret = gst_va_handle_context_query (GST_ELEMENT_CAST (decoder), query,
display);
gst_object_unref (display);
return ret;
}
return GST_VIDEO_DECODER_CLASS (parent_class)->sink_query (decoder, query);

View file

@ -186,19 +186,20 @@ gst_va_ensure_element_data (gpointer element, const gchar * render_device_path,
/* 1) Check if the element already has a context of the specific
* type.
*/
if (gst_va_display_found (element, *display_ptr))
if (gst_va_display_found (element, g_atomic_pointer_get (display_ptr)))
goto done;
_gst_context_query (element, "gst.va.display.handle");
/* Neighbour found and it updated the display */
if (gst_va_display_found (element, *display_ptr))
if (gst_va_display_found (element, g_atomic_pointer_get (display_ptr)))
goto done;
/* If no neighbor, or application not interested, use drm */
display = gst_va_display_drm_new_from_path (render_device_path);
*display_ptr = display;
gst_object_replace ((GstObject **) display_ptr, (GstObject *) display);
gst_object_unref (display);
gst_va_element_propagate_display_context (element, display);
@ -231,11 +232,9 @@ gst_va_handle_set_context (GstElement * element, GstContext * context,
}
if (display_replacement) {
GstVaDisplay *old = *display_ptr;
*display_ptr = display_replacement;
if (old)
gst_object_unref (old);
gst_object_replace ((GstObject **) display_ptr,
(GstObject *) display_replacement);
gst_object_unref (display_replacement);
}
return TRUE;