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 684832 - videodecoder: Takes stream lock in query function
videodecoder: Takes stream lock in query function
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.0.0
Other Linux
: Normal normal
: 1.0.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-09-25 22:52 UTC by Olivier Crête
Modified: 2012-10-01 18:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videoencoder: clip input buffers to current input segment (10.11 KB, patch)
2012-09-25 22:52 UTC, Olivier Crête
needs-work Details | Review
videodecoder: Also use the object lock to protect the output_state (3.99 KB, patch)
2012-09-29 00:10 UTC, Olivier Crête
accepted-commit_now Details | Review

Description Olivier Crête 2012-09-25 22:52:27 UTC
Created attachment 225183 [details] [review]
videoencoder: clip input buffers to current input segment

The video decoder takes the stream lock in the query function because that's what protects the output state, follows a patch to protect it with the object lock instead.
Comment 1 Sebastian Dröge (slomo) 2012-09-26 07:01:18 UTC
Review of attachment 225183 [details] [review]:

Looks good in general, please push after addressing:

::: gst-libs/gst/video/gstvideodecoder.c
@@ -2640,3 @@
   state = _new_output_state (fmt, width, height, reference);
 
-  GST_VIDEO_DECODER_STREAM_LOCK (decoder);

I'm pretty sure set_output_state() should also be protected by the stream lock.

@@ +2842,3 @@
   klass = GST_VIDEO_DECODER_GET_CLASS (decoder);
 
+  GST_OBJECT_LOCK (decoder);

And the same for negotiate_default()
Comment 2 Olivier Crête 2012-09-29 00:10:01 UTC
Created attachment 225366 [details] [review]
videodecoder: Also use the object lock to protect the output_state

This is a much less invasive patch. Also probably makes 62c111f1e obsolete

Hold both the stream and the object lock to modify the output_state,
this way it can be safely modified while hold either one or the other.

Also, only hold the object lock in the query
Comment 3 Sebastian Dröge (slomo) 2012-09-29 13:14:51 UTC
Comment on attachment 225366 [details] [review]
videodecoder: Also use the object lock to protect the output_state

Yes, looks much simpler. Please push
Comment 4 Olivier Crête 2012-10-01 18:45:35 UTC
Merged

commit 531a5af30c17fe846076eefd6f51109df8ca00a9
Author: Olivier Crête <olivier.crete@collabora.com>
Date:   Fri Sep 28 20:07:43 2012 -0400

    videodecoder: Also use the object lock to protect the output_state
    
    Hold both the stream and the object lock to modify the output_state,
    this way it can be safely modified while hold either one or the other.
    
    Also, only hold the object lock in the query
    
    https://bugzilla.gnome.org/show_bug.cgi?id=684832