[MPlayer-dev-eng] [PATCH] avoid useless full xvmc reinit with -fixed-vo if possible

[Ivan Kalvachev]

2007/2/18, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Sun, Feb 18, 2007 at 12:16:40PM +0200, Ivan Kalvachev wrote:
> > 2007/2/18, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > >Making this entirely internal to the vo would mean duplicating the code
> > >for each. But I will update the patch to make the needed changes in
> > >video_out.c instead of vf_vo.c
> >
> > I don't like it, either.
> > The thing is that the vo already knows and stores the dimensions.
> > Instead of making check by itself, the vo checks for flag that is set
> > by vf_vo.c.
> > I'm trying to say that instead of duplicating:
> >
> > if (vo_width==width && vo_height==height && vo_fmt==pix_fmt)
> >
> > we would be duplicating
> >
> > if (!(flags & VOFLAG_SAME_INPUT))
> >
> > The difference is that in the second case we have
> > + 3 new variables
> 3 new variables in video_out.c, in the former case 3 new variables in
> each vo that supports it.

Each vo stores the width and height it gets by config(), so you are
just adding more cruft.
In vo_xvmc, these are image_height, image_width. I confess that I
don't store the format, but that would add one variable ;)

> > + new code to update and compare these variable

The worst thing is that the new code is not smart enough.
e.g. It doesn't handle -zoom cases, (where the vo uses swscale
directly and on aspect change would have to initialize), so such vo
should either ignore it completely or check the zoom flag too.

Similar is the case where e.g. svga/vesa vo initializes modes that are
bigger than requested, so the range of possible resolution that
doesn't require mode reinitialization is much bigger.

So in short your code is useless BLOAT.
It creates a whole new infrastructure for thing that is implemented in ONE LINE.


Anyway. There is something mode fundamentally wrong in your patch. If
you had tested it with -loop 10 -fixed-vo, you'd quite easy would get
an abort(), due to surface deprivation.

Because MPlayer lacks release() mechanism I had to implement such
locking for the surfaces. I also lock surfaces that are not yet
displayed or still visible (xvmc have the bad habit of keeping frame
long after another is requested) to prevent visible image corruption.
There is second locking for subtitle rendering as the most common
method involves basically re-rendering into another surface.
These lockings should be cleared, when reconfig()-ing

There is also queue options, so the queue should be zeroed too...