[MPlayer-dev-eng] [PATCH] Patch for better pause support.

[Robert Cummings]

On Thu, 2008-09-25 at 20:03 +0200, Reimar Döffinger wrote:
> Hello,
> On Thu, Sep 04, 2008 at 12:45:58PM -0400, Robert Cummings wrote:
> > I took a look through the code some more. The is_paused command can use
> > mpctx->osd_function == OSD_PAUSE instead to get it's pause status (patch
> > attached).
> 
> Having looked at the overall code, while I do not exactly like how the
> "osd_function" is misused I think this is the correct way.
> There are just two uglynesses:
> 1) I think it should be done via the get_/set_property interface IMO
> 2) it actually reports the previous state (admittedly that is due to the
> changes I suggested in part).
> 
> > This would mean setting mpctx->was_paused can be skipped
> > within the pause_loop. That said, I still think for consistency that
> > mpctx->was_paused should be changed to mpctx->is_paused and this should
> > still be set within the pause_loop (patch attached - merge with
> > pausing_keep_force patch).
>
> Actually, I decided it is best for consistency and to minimize the
> changes to set it _before_ entering the pause_loop.

Thank you for the work you have done. I certainly appreciate the
patches. I only have one observation with respect to the process here
and hope to not sound like I'm trolling. It's seems to me that the
philosophy of "minimize the changes" is orthogonal to "properly fixing"
the problem. I think my original patch without pausing_keep_force (while
certainly not perfect) was superior in the sense that it provided the
desired functionality without forking the command system into a third
mode. Now there are three modes: standard, pausing_keep, and
pausing_keep_force; with a progression to seeming ever more hackish and
patchy... all to "minimize the changes" at the expense of increasing WTF
factor :/

> Attached is an untested patch that should implement
> get_property pause
> It misses the documentation part though.

Thanks, I'll give it a run.

Cheers,
Rob.
-- 
http://www.interjinn.com
Application and Templating Framework for PHP