[MPlayer-dev-eng] [PATCH] Direct3D VO better handling of uncooperative video adapter

[Reimar Döffinger]

On Tue, Feb 03, 2009 at 12:43:03PM +0200, Georgi Petrov wrote:
> On Tue, Feb 3, 2009 at 12:24 PM, Reimar Döffinger
> <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> > Also have you thought about but ensure_..._created/ensure_..._freed
> > system? Currently it is possible (though unlikely) that we have e.g. a
> > valid d3d_device but not video texture.
> 
> Hmm, most likely this is true. I'll have to review the whole code very
> carefully or even change a little bit the code to remove those
> half-initialized possible states.

Well, my idea with those ensure_ function was to do exactly that, but in
a way that does not lead to the same kind of IMO chaotic and hard to
understand code we currently have (mostly this would be due to
documenting the possible initialization states and have
consistently-named functions to transition between them).
Also by using ensure_..._created a context would be recreated as soon as
possible/needed, not only in flip_page.

> The patch as it is now is nevertheless needed, because it only adds
> checks, which are good to have anyways.

Yes, I don't object to it, my suggestion was just a more complete way,
and one that is easier to verify for correctness.

> >> @@ -857,21 +862,16 @@
> >>  static void flip_page(void)
> >>  {
> >>      RECT rect = {0, 0, vo_dwidth, vo_dheight};
> >> -    if (FAILED(IDirect3DDevice9_Present(priv->d3d_device, &rect, 0, 0, 0))) {
> >> +    if (!priv->d3d_device ||
> >> +        FAILED(IDirect3DDevice9_Present(priv->d3d_device, &rect, 0, 0, 0))) {
> >>          mp_msg(MSGT_VO, MSGL_V,
> >> -               "<vo_direct3d>Video adapter became uncooperative.\n");
> >> -        mp_msg(MSGT_VO, MSGL_ERR, "<vo_direct3d>Trying to reinitialize it...\n");
> >> +               "<vo_direct3d>Trying to reinitialize uncooperative video adapter.\n");
> >>          if (!reconfigure_d3d()) {
> >> -            mp_msg(MSGT_VO, MSGL_V, "<vo_direct3d>Reinitialization Failed.\n");
> >> +            mp_msg(MSGT_VO, MSGL_V, "<vo_direct3d>Reinitialization failed.\n");
> >>              return;
> >>          }
> >> -        if (FAILED(IDirect3DDevice9_Present(priv->d3d_device, &rect, 0, 0, 0))) {
> >> -            mp_msg(MSGT_VO, MSGL_V, "<vo_direct3d>Reinitialization Failed.\n");
> >> -            return;
> >> -        }
> >
> > Why are you removing it? I guess it can't work, but if so it should be
> > done in a separate patch with the reason explained in the commit
> > message.
> 
> In flip_page() it was the same - a Present was issued on a possibly
> invalid d3d_device and THIS was the ultimate reason MPlayer used to.

Yes, but if that was the only issue, the right solution would be to add
priv->d3d_device && ...

> In this function I have removed the second call to Present, as it
> won't have any frame to present before the next render_d3d_frame is
> called.

I thought so, then removing it is of course the right method.