GNOME Bugzilla – Bug 749531
xvimagesink: refactor code to remove unnecessary mutex lock/unlock
Last modified: 2015-05-18 12:12:40 UTC
When xvimagesink->xwindow is not present, there is a seperate condition to check it and prepare window handle, and mutex lock/unlock is being called seperately for the same. This is not needed. The same condition can be added while creating the new instance of xwindow.
Created attachment 303506 [details] [review] remove unnecessary mutex lock/unlock
I think you misunderstood what this code does. The prepare_window_handle() will post a message on the bus that gives the application a chance to supply its own window handle. Applications might intercept the message in a sync message handler and then call the gst_video_overlay_set_window_handle(). The the whole point of this code is to first give the application a chance to set its own window, and then in the second if() we create our own if we didn't get one from the application. Furthermore, prepare_window_handle() must be called with the lock UNLOCKED otherwise we get a deadlock if application code calls one of our functions that takes the lock again (like _set_window_handle). Please re-open if I missed something though, thanks.
Hmm it does make sense.. But what i didn't understand is why do we need to lock just to check if window is not null and then again unlock it.. it is not like we are operating on the window.. Or is it needed to lock even for just checking?
You need to lock whilst checking as well, because it's possible that the window is just being set from the application thread at the same time. The extra locking should not really be a problem here, the set_caps function is not called very often.