[MPlayer-dev-eng] [PATCH] OSD menu #if 0 define remove and pause key bug fix

[Alban Bedel]

On Sat, 10 Nov 2007 14:06:11 +0800
Ulion <ulion2002 at gmail.com> wrote:

> 2007/11/10, Alban Bedel <albeu at free.fr>:
> > On Sun, 4 Nov 2007 16:16:53 +0800
> > Ulion <ulion2002 at gmail.com> wrote:
> >
> > > Hello,
> > >
> > > In libmenu/vf_menu.c, there's a #if 0 comment out 4 lines to
> > > prevent a pause key bug (without osd menu showed but running
> > > mplayer with -menu parameter), it from a link in 2003:
> > > http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2003-March/017281.html
> > > The comment out did prevent the pause key problem, but it kill
> > > the osd menu pausing display (select pause from osd menu, after
> > > that, display will no longer update but you can move the menu
> > > selection invisible). Set #if 0 to #if 1, you will see what's the
> > > proper behavior after select pause from osd menu.
> > > The real reason cause the pause key problem descripted in the old
> > > link, is that cmd filter handle all MP_CMD_PAUSE no matter whether
> > > currently osd menu is displayed on the screen, it should only
> > > handle MP_CMD_PAUSE when osd menu is displayed. Here's the real
> > > fix for that problem.
> >
> > Sorry I haven't much time for all this atm. Your patch is fine for
> > the current code, but this is not the real fix.
> >
> > The pause filter was meant to work all the time to also eventualy
> > allow setting volume, etc when paused but not displaying the menu.
> > The bug was introduced in R8245, the added if is not at the right
> > position so it also skip code meant to run at every frame (like the
> > pause stuff) and things go havoc when the menu is not displayed. It
> > should be enouth to just move the if further down to just enclose
> > the drawing code, and get the original behaviour back.
> >
> >         Albeu
> >
> 
> I get your meaning that the pause filter should be called every frame
> to get the pausing frame so after paused by user by keyboard he can
> still call out the osd menu and with correct background displayed. I
> made a patch accounding what you said move the if down before draw
> here, it works.
> But two thing I don't understatnd:
> 
> 1. You said 'The pause filter was meant to work all the time to also
> eventualy allow
> > setting volume, etc when paused but not displaying the menu. ', but
> > the pause filter is no matter with setting volume, that work is
> > down in mplayer.c there's a loop which peek events in the cmd
> > queue, without osd menu, it works, with osd menu, it has nothing
> > different, also works when osd menu is not showing. When osd menu
> > showed, the mp_input_key_cb steals all cmd from input, so only
> > hardcoded osd menu keys will work when osd menu is showing. If you
> > mean when osd menu is showing, besides hardcoded osd menu keys,
> > other key bindings not conflict with the hardcoded
> key should also work, that will need more work to do.

No, I mean using the normal binding while paused but outside of the menu
and still having the OSD beeing drawn correctly. I know that more
changes are needed, but for a correctly working OSD the first step is
having the last video frame captured as the pause filter is doing.

> 2. In the pause filter there's a delay hack, why it's there? Without
> the delay, seems everything works fine. It is imported at the
> beginning of svn log, so may you explain why it's there and whether we
> can remove it now safely? I don't like the delay, specially when osd
> menu is not showing, it's not necessary, isn't it?

iirc (that was really a long time ago) I had some extra code in main()
to allow seeking when paused. It would go out of pause, seek and pause
again. In such case sometimes it ended up showing the wrong frame
(one from the old position), so I added this delay hack to ensure it
showed one of the new ones. So yes, it can be removed. Or go2pause
could be changed to get something like this:

go2pause > 2  : skip the frame, go2pause--
go2pause == 2 : capture the frame and resend a pause cmd, go2pause = 1
go2pause == 1 : let the resent pause go through, go2pause = 0

That would still allow the cmd filter to choose to skip some extra
frame if needed while getting us rid of this hack.

	Albeu