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

[Reimar Döffinger]

On Tue, Feb 03, 2009 at 12:57:38PM +0200, Georgi Petrov wrote:
> >> >> @@ -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 && ...
> >
> 
> No. The current code (before the patch) is this:
> 
> if (FAILED(IDirect3DDevice9_Present(priv->d3d_device, &rect, 0, 0, 0)))
> 
> It's flaw is not checking if priv->d3d_device is NULL.
> 
> After the patch it is:
> 
> if (!priv->d3d_device ||
>     FAILED(IDirect3DDevice9_Present(priv->d3d_device, &rect, 0, 0, 0)))

Uh, I am talking about the second Present call which you removed.
Since it can not work _in general_ (which I wanted you to confirm)
it is correct to simply remove it, but to avoid the crash adding
"priv->d3d_device && ..." would have been enough.
Again, I am talking about the _second_ Present, for the first there is
no question.