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

[Andrew Calkin]

On Sun, Nov 11, 2007 at 08:18:17PM +0800, Ulion wrote:
> 2007/11/11, Alban Bedel <albeu at free.fr>:
> > 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.
> 
> I still not got exactly what you mean. You said 'OSD' standalone with
> the word 'menu', did you mean osd, or osd menu? And you said 'outside
> of the menu', did it mean bindings outside of hardcoded in libmenu or
> it mean when menu is not showing.
> At least I got the meaning that capture the last frame as pause frame,
> then I will have a look to the current code to see whether we can do
> better for it. Indeed I can see by my eyes that the pause filter
> currently not correctly capture the last frame before pause, it
> normally captures a frame before the last frame.
> 
> >
> > > 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.
> >
> 
> I check the pause filter code, found it will have problem when filter
> the PAUSE cmd while there's cmds in the queue. Current vf_menu queue a
> pause cmd to the cmd queue, it's not clean. And also now cmd has a
> pausing property, which are not handled by pause filter by now.
> Anyway, I will try to dig it deeper to find some clean way for the
> purpose -- copy the last frame before pause.
> 
> btw: Any response to the property version of select sub by source
> patch: http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2007-November/054683.html

I'm wondering, could this functionality of the pause filter be used when
decoding single images, for a sustained display? e.g. if an image file
is to be shown on the screen in a sustained fashion, then using 
"-loop 0" results in the image being decoded multiple times, which for
large files on slow h/w, results at best in a flickering display. Could
this pausing functionality be used to open an image in a paused state,
and then allow access to the OSD menu? If so, then further applications
related to showing still images could be easily done, without needing to
decode the image multiple times. Possibly a patch to allow a flag or
config option to open single frame images (e.g. not animated gifs or the
like) in paused mode would then be feasible?

//Andrew