[MPlayer-dev-eng] New Main() loop

[Ed Wildgoose]

>You have to be very carefull with such requests. We are here somewhere
>at the edge to real time applications. Currently we need to get called
>every 1/fps, which is around 30ms. Inacurancies of the sleep resp call
>time of 5-10ms start to get noticable, anything above 20ms is very bad.
>10ms is already considered soft realtime.
>An event naive aproach to an event loop would reduce sleep times
>greately as the next event may be scheduled at any time. Thus moving
>us further into the direction of hard realtime. So your i
>"not such a big change" will get quite big to handle those events
>in an intelligent way by not requesting too small sleep times,
>not introducing "have to wait for 1ms" spin loops that waste
>precious cpu time and still working in 99% of all cases.
>  
>

No complaints there.  To be honest, perhaps what I had in mind was more 
of a halfway house to a proper event look.  Something which looks more 
like the current main loop, but with "wait breaking" might be more 
appropriate.  Consider perhaps something more like breaking any video 
waits above Y ms into multiple smaller waits.  Also, handling ultra 
small waits using some other mechanism, for example (haven't thought 
this through completely) blocking on writing some small amount of 
audio.data.

I certainly don't anticipate introducing, or needing to introduce 1ms 
waits or silly things like that.  I guess that point is that there are 
hard realtime events like waiting for frame redraw time, and soft wait 
states like keyboard events (where you basically want to break a longer 
wait into several smaller ones of greater than some lower limit number 
of ms).

>>I guess a useful patch would also be to adjust the logic so that 
>>frameseeks caused the frame draw to become instantly due, and not have 
>>to wait a frame interval to draw them.  Any reasons why this change in 
>>behaviour would break something subtle?
>>    
>>
>
>Uhmm.. how many assumptions of the code will this break ?
>Please keep in mind that playing a video file is not just
>filling the audio buffer and displaying a picture once in a while.
>It's a lot more complex (mplayer.c wouldnt be nearly 4k lines otherwise).
>  
>

Fair enough.  This point has been made to me a few times, so I will 
retract it completely now...!

>I just had a look at your patch again, and it really looks like
>a no-brainer, but i somewhat feel there is more to it. The main loop
>has to keep a lot of things in mind and i somewhat think that you left
>out a few of those.
>  
>

Thanks.  As has been pointed out, it changes the code path in two 
situations, both where you don't have enough audio for your playback 
buffer to avoid underrun.  The one that affects me is normal framerate, 
eg 1/25 ish but you still don't have enough audio buffering.  I don't 
think anything else gets worse here with this patch - it shouldn't 
change the code path for anyone else (since they would be in the usleep 
state and not doing anything), and you couldn't use mplayer at all 
before without it. 

There is a second situation though which is with suprisingly low frames 
like 1 frame per second.  In this case you would have had fast 
fast-forward before, but without video updating. Now you will find that 
your keyboard response rates are determined by the frame-rate, etc.  
This is caused by the "if ... continue" line further down the main() 
loop.  By all means omit  that if you think that this behaviour is 
undesirable, or perhaps (better) we could make the test distinguish 
between low and high rate video, for example if we are breaking a 50ms 
or longer wait then do the rest of the loop normally, otherwise go back 
to the top of the loop as per the patch.  This is a trivial patch, but I 
can resubmit with this ammendment if you prefer?

Finally, the best way to do this, is as I suggested above, and move to a 
more advanced state machine, or perhaps call it an event loop.  What I 
mean by that is ideally you would want to continue to loop through the 
audio loop while waiting for video, but only up to a certain length of 
time (I think it is set at 30ms in the current code?), after that you do 
a scan through the rest of the main loop, etc.  This I think gives ideal 
behaviour and can be done in the current main loop by adding in an extra 
variable to track how long we have been spinning for and release to run 
the rest of the main loop after a period.  However, by tidying things up 
a little, this is basically all I was thinking off when I was suggesting 
something more like an event loop anyway (but obviously alarmed people 
with the way I suggested it).

Does this take us forward at all?

Ed W