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

[Georgi Petrov]

On Tue, Feb 3, 2009 at 12:24 PM, Reimar Döffinger
<Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> On Tue, Feb 03, 2009 at 11:41:10AM +0200, Georgi Petrov wrote:
>> > I think it might be because reconfigure_d3d has already released the device.
>> > In that case, you do not need that uncooperative variable but can just check for
>> > if (!priv->d3d_device) ...
>>
>> Great. I submit much simpler patch, which works very well and changes
>> only a few lines to to the same. I check exactly for
>> !priv->d3d_device. Please, take a look.
>
> You really shouldn't change the mp_msgs in the same patch, it makes it
> needlessly hard to understand.

Ok, I'll try to differentiate patches better. I'v also written to
Diego for SVN access, so I can start learning how to submit good/valid
patches.

> 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.

The patch as it is now is nevertheless needed, because it only adds
checks, which are good to have anyways. The problem before was that in
render_d3d_frame and draw_slice no checks were performed if
priv->d3d_device really exists and tried to use macros, which take
d3d_device as argument and call pointers to functions FROM it. Imagine
what is happening if it's not valid.

>> @@ -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.
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.