[MPlayer-dev-eng] [PATCH] VFCTRL_PERIODIC_UPDATE

[Jason Tackaberry]

On Fri, 2007-01-12 at 08:07 +0200, Uoti Urpala wrote:
> > 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.

Not true.

On open, the filter calls mp_input_add_cmd_filter() to attach a callback
that gets called on slave commands.  So for example:

        static int open(vf_instance_t *vf, char *args) { 
           ...
           mp_input_add_cmd_filter((mp_input_cmd_filter)cmd_filter, vf->priv);
           return 1;
        }
        
        static int cmd_filter(mp_cmd_t *cmd, int paused, struct vf_priv_s *priv) {
           if (cmd->id == MP_CMD_VF_OVERLAY) {
              ...
              return 1;
           }
           return 0;
        }
        
Then the vf's control handler calls mp_input_get_cmd(0,0,1) to peek at
the input fds.  If a slave command is pending, cmd_filter gets called
indirectly by the input layer.  If it returns 0 (which it does when it
is not an overlay command), the command gets requeued and is handled as
usual inside mplayer.c


> 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.

And this would have to be done during the pause loop too, keeping in
mind that overlay slave commands mustn't unpause a paused video.
Wouldn't this require, inside the "key_events" module, we handle the
overlay slave command, check if was_paused=1, set osd_function=OSD_PAUSE
again, and jump back to the pause loop?  (If we don't jump, wouldn't it
eat a frame?)

I have a feeling the timing loop case would be more insidious, but
without actually implementing it this way I'm not sure.  Maybe you can
suggest a cleaner way for the pause loop case, but is it as clean as the
cmd_filter approach?  And if you find the cmd_filter way unelegant (or
even less elegant), why?


> > 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.

All right.  So the idea is to interrupt mplayer's timing loop and pause
loop upon activity of some fd, and if there is activity, call
vf->control().  Though with this implementation VFCTRL_PERIODIC_UPDATE
ought to be renamed to something more representative, like
VFCTRL_WAKE_UPDATE.  (Please suggest something better.)

It seems that this belongs in the input system.  Something like
mp_input_wake_add_fd() and mp_input_wake_remove_fd()?  vf_overlay would
add stdin since it it interested in waking on slave commands.  In the
control() function for the filter, it could still call
mp_input_get_cmd(), but at this point would know there is something
waiting.

Your thoughts?

Thanks,
Jason.