[MPlayer-dev-eng] [PATCH] VFCTRL_PERIODIC_UPDATE

[Uoti Urpala]

On Thu, 2007-01-11 at 23:35 -0500, Jason Tackaberry wrote:
> On Fri, 2007-01-12 at 05:42 +0200, Uoti Urpala wrote:
> > If it's a slave command that triggers the timing then it sounds like the
> > slave input needs to be read and parsed, and that might cause new
> > problems since then you need to be able to somehow handle ALL slave
> > commands in any location that could sleep. Your previous patch did not
> > read slave input between the sleeps in main timing; would something
> > (what?) have triggered the filter, or was that another error in the
> > patch?
> 
> It's not a mistake.  The slave command is handled in the filter.  To do

That means the filter would have to read from the slave fd and be able
to handle or queue ANY slave command (since it can't know without
parsing whether the command concerns the filter). That sounds quite
ugly.

> otherwise would require a scary mishmash of gotos inside mplayer.c,
> jumping back and forth between the "key_events" section and the
> pause/timing loops, as you have eluded.

As I wrote in the part you didn't quote:

If you need to handle slave commands immediately then it's probably
better to exit the current timing loop when there's control input and go
around the main play loop to read+handle the input.

This does not require "a scary mishmash of gotos". See the current code
in mplayer.c using the frame_time_remaining variable.

> But we're getting ahead of ourselves here.  If you do have a problem
> with how I'm handling the commands in vf_overlay, we can hash that out
> when I submit that patch.  I had hoped to simplify things by breaking up
> the patches and deal with them independently.  Certainly it's useful to
> talk about vf_overlay in the sense that is a user of this vfctrl, but
> they are not inextricably linked.

Since the desired timing and slave-command interaction properties depend
on the filter the update mechanism cannot be discussed completely in
isolation. And especially if the patch is implemented in a "I know this
isn't a good way to do it but it's easier for me and enough for the
current purpose" fashion then exactly what the purpose is does matter.

> The idea of VFCTRL_PERIODIC_UPDATE is to provide filters the ability of
> updating the frame while paused and inside the timing loops.  So it

However if the actions needed for updating include timing or outside
interaction such as slave commands then trying to keep it all inside a
normal filter call probably isn't a good way.

> seems there's a question of how to do that without polling.  Is that
> really so important though, given that polling as the patch proposes
> doesn't add any appreciable overhead to critical areas for the case
> where there is no VFCTRL_PERIODIC_UPDATE consumer?

Saying that the patch doesn't do anything (bad) as long as you don't try
to use the functionality isn't exactly a strong argument for including
it. When the polling is done it might not be a performance critical area
for the MPlayer process itself, but it harms other processes on the
machine. And in my opinion the practical problems caused by polling do
not need to be huge for a non-polling implementation to be preferable;
polling IS inferior in several ways, and it doesn't even look like the
polling version would be any simpler overall given a proper non-polling
implementation.