On Wed, 2 Mar 2016, Frediano Ziglio wrote:
> >
> > Signed-off-by: Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
> > ---
> >
> > In theory this could be needed by the next patch.
> >
>
> Are you saying that now compression is done in another thread?
Nothing has changed with regards to threading. My understanding is that
GStreamer always forks its own thread(s) to handle the pipeline.
The difference is that before the pipeline elements were not touching
the RedDrawable refcount and now they do.
Furthermore, as explained in the next patch, I think that all the
incrementing and decrementing of the RedDrawable refcount will happen
during the encode_frame() call, i.e. there will be no race condition
since the main thread will be blocked in the gst_app_sink_pull_sample()
call.
However this relies on reasonable assumptions on how the pipeline
elements behave, for instance that the videoconvert element will
actually have to convert the frame and thus will release the input
buffer as soon as that's done. But the goal of changing from the
previous approach to this one is to get rid of these assumptions and
thus ensure this works even if the buffer we push to the pipeline is
freed some time after gst_app_sink_pull_sample() returns. If that
happens then we could get a race condition. Hence this patch.
[...]
> > void red_drawable_unref(RedDrawable *red_drawable)
> > {
> > - if (--red_drawable->refs) {
> > + gint old_refs;
> > + do {
> > + old_refs = red_drawable->refs;
> > + } while (!g_atomic_int_compare_and_exchange(&red_drawable->refs,
> > old_refs, old_refs - 1));
>
> This code looks a bit odd. Usually there is a function to do it. 486
> introduced xadd instruction and GCC have atomic built-ins.
> Personally I would like to have a syntax that allows to use C11 instead of
> some library specific ones.
This is loosely modeled after the GObject refcount code, though that
code is much more complex due to floating and weak references. I think I
wanted to use g_atomic_int_dec_and_test() in another context and
concluded I could not use it there, but here it seems like it would work
just fine.
We could use the C11 atomic operations directly but GLib's gatomic.h
header does that for us and makes it more portable.
--
Francois Gouget <fgouget@xxxxxxxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel