[mythtv] [PATCH] Fix for crash using OpenGL vsync

[Doug Larrick]

Isaac Richards wrote:
> On Thursday 01 July 2004 08:02 am, Doug Larrick wrote:
> 
>>The attached patch fixes a crash when starting to watch the second (or
>>subsequent, if lucky) recording when using OpenGL vertical retrace sync.
>>  I believe the underlying cause is a bug in the nVidia OpenGL
>>implementation; in fact, some searches indicate this same bug existed in
>>some older SGI OpenGL implementations so maybe they've simply used a
>>reference implementation for this part of the standard.
> 
> 
> Patch doesn't feel right.  That usleep in there is going to cause problems, I 
> think..  

Which one?  I think you're not understanding what I've done here. 
There's a usleep that waits after I launch the thread, to avoid a race 
for s_glsync_good.  Then there's the usleep in the sync thread's loop 
itself, which is critical so the player threads have a time window in 
which to acquire the sync_mutex lock.  This loop (after "Loop forever in 
this thread") repeats once per vertical refresh (16667 usec for 60 Hz). 
  For roughly 14667 usec of this loop, it holds the mutex while sleeping 
inside glXWaitVideoSyncSGI.  When this returns, we know we're in 
vertical retrace.  We release the mutex and usleep for 2000 usec, in 
which period any video playback thread(s) may acquire the mutex briefly. 
  At the time when a playback thread is able to acquire this mutex, it 
knows it's (approximately) in vertical retrace.  I used the value 2000 
because it's around the same precision as ExAVSync tries to maintain.

> It may be a slightly cleaner workaround to send a message to the main UI 
> thread to tell it to do something with GL after playback has ended, or to 
> spawn off a class + thread that can receive a similar message.  Still have a 
> thread 'using' GL that way, and it'd not change any of the current logic.  
> Thoughts?

The "thread 'using' GL" has to do an operation once per video refresh. 
And it needs to be tightly synchronized to video playback -- I'm 
concerned that sending an inter-thread message is too high-latency (and 
more importantly, variable-latency) to do any good here.  I don't think 
the main UI thread is suitable.

Believe me, I spent almost a month trying easier approaches.  The thing 
is, to work around the OpenGL driver bug I discussed in my original 
message, I need to make these glXWaitVideoSyncSGI calls from a thread 
that never goes away, or the *second* attempt to start up the vsync 
system fails (crashes).

I also thought about launching an external program that talks to OpenGL 
and communicates with mythfrontend via a pipe (essentially emulating the 
old nVidia vsync), but that seems fragile.

> Also, is the GL code cleaning up after itself properly?  I'm seeing a call to 
> XOpenDisplay + a couple XCreate??, but nothing to get rid of em..

The sync thread runs continuously from the start of the first video 
playback until the end of the world (mythfrontend exit), because it has 
to.  There's nothing to clean up until program exit, at which point it 
doesn't matter anymore.  I suppose I could set another flag so it sleeps 
more and does less work when there's no video playback, but it consumes 
so little CPU to begin with I hardly think it's worth it.  On my 
(admittedly fast) system, having this thread running consumes somewhere 
near 0.05% of the CPU. I wrote a quick standalone program that is just 
the same loop, ran 10 copies of it, and the CPU meter bounces between 0% 
and 1% consumption.

I'm not really any happier about this state of affairs than you are -- I 
wish the driver worked properly, I wish it were open source so I could 
have debugged it instead of guessing for a month and doing Google 
searches, I wish the old simpler /dev/nvidia0 vsync interface still 
worked, and I wish vsync.c was vsync.cpp so I could use a QWaitCondition 
object and make the intention of the code clearer.  But this 
dedicated-thread approach is the cleanest way I can think of to achieve 
the objective of a working sync to vertical retrace within the existing 
framework.

-Doug
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : http://mythtv.org/pipermail/mythtv-dev/attachments/20040702/8b1bbe35/signature.pgp