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

[Nico Sabbi]

Il Sunday 11 November 2007 16:28:02 Ulion ha scritto:
> 2007/11/11, Andrew Calkin <andrew.calkin at gmail.com>:
> > 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/200
> > > > > > >3-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-Novemb
> > >er/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
>
> As you see, the pause filter still break things and it not capture
> the last frame as it was expected by now. And if the put_image
> called by filter not get called (because video not update) after
> the filter the pause cmd, it will not pause at all, also it will
> have no chance to get any frame, unless it always capture frames,
> but that's just the pause filter try to avoid...


guys, please learn to quote. I'm complaining because
trying to read each of your untrimmed and kilometric reply is
overly time consuming