[Bug 30557] winegstreamer broken with >=glib-2.32.0

[wine-bugs at winehq.org]

https://bugs.winehq.org/show_bug.cgi?id=30557

--- Comment #118 from Andrew Eikum <aeikum at codeweavers.com> ---
(In reply to Sebastian Lackner from comment #117)
> While reviewing patch 2/2, I've found a couple of things which probably
> should be fixed. I am aware that its a very early proof of concept version,
> but decided to share my feedback nevertheless:
> 

Thanks very much for the review. It needs a little more thought and cleanup,
but otherwise I think it's in OK shape for review.

> - If wine_get_current_teb() returns != NULL, pthread lock/condition variable
> functions are executed on uninitialized memory.

Yup, thanks.

> - Instead of CreateThreadpoolWork, I would suggest to use
> TrySubmitThreadpoolCallback(). It basically does the same in your case in a
> single command, instead of three.

OK, I'll look at that.

> - "Small" functions should probably be executed directly in the dispatcher
> thread, to avoid the additional overhead. Also, it probably should be used
> as a fallback if creating a threadpool task fails.

This one I'm not sure about. I like the consistency of ALL callbacks going
through call_cb, so we only need to avoid Wine code in gst_cbs.c and call_cb.
It may be important if any of the cbs are on performance-critical paths,
though. I'll do some analysis.

> - The thread handle returned by CreateThread is currently leaked. It would
> also be useful to implement a mechanism to terminate the dispatcher thread
> when its no longer required.

I thought about this. One thing is that winegstreamer never unloads (see
Gstreamer_init), so having one extra thread idling in the background doesn't
sound too terrible to me. Otherwise we have to determine when all objects with
callbacks have been destroyed. I'll take another look at it, anyway.

> - Functions and global variables in gst_cbs.h should be marked as
> DECLSPEC_HIDDEN.

I wanted to avoid including Windows headers in gst_cbs.{h,c}, so any Windows
code calls would fail to compile. Unfortunately DECLSPEC_HIDDEN lives in a
Windows header. Probably I can just include a Windows header, but I liked the
cleanliness of avoiding it :)

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.