[MPlayer-dev-eng] [PATCH] Direct3D libvo driver

[Georgi Petrov]

On Sun, Nov 16, 2008 at 11:34 PM, Reimar Döffinger
<Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> On Sun, Nov 16, 2008 at 10:04:10PM +0200, Georgi Petrov wrote:
>> +BOOL gIsPaused;              /**< TRUE  = Movie is paused,
>> +                                  FALSE = Movie is not paused */
>
> I'll just mention it once: that whole BOOL TRUE/FALSE stuff is very much
> against the concept of C, there is a clear "BOOL" type called int, and 0
> is false and everything else is true.
> I can't help but feel reminded of that good old Java joke of doing
> "if (((a == b) == true) == false)"
>

Fixed.

> Note: for proper panscan support this assumes that StretchRect can
> handle left/top to be negative and right/bottom larger than the
> destination.
> The documentation is useless as always, the people writing MSDN stuff have never
> realized the concept of "corner cases" and the usefulness of documenting
> them...
> Anyways, to test that corner case use "-fs -panscanrange -2" and use the
> w/e keys to zoom in/out. -vo gl should probably behave correctly.
>
>

I fully agree on the documentation side. It's awful. I have to test so
many things and search endless forums just to realize something not
documented at all...

Unfortunately, StretchRect doesn't support negative or larger
destination coordinates. Instead I'll have to pass it source RECT
which is smaller part of the original Offscreen Surface and resize
that smaller (zoomed) rect to the destination. I'll do it these days -
it'll require a lot of testing.


>> +    if((mpi->flags & MP_IMGFLAG_DIRECT) ||
>> +       (mpi->flags & MP_IMGFLAG_DRAW_CALLBACK))
>> +        return VO_TRUE;
>
> The MP_IMGFLAG_DIRECT is no good here, firstly as long as you do not
> implement direct rendering it can never happen anyway, and if you do
> implement it the code here basically would be the same as in the
> "normal" case except you skip the memcpy stuff (since the decoder would
> have written directly into stLockedRect.pBits itself).
>

Fixed (I think so - take a look). I have only a quick question: when I
implement DR, does the decoder know about the difference in stride vs
scanline width like memcpy_pic does? And why isn't DR turned on by
default for drivers that support it?

> Not sure how slow it is nowadays, but I'd guess it would be nice if the
> D3D context wouldn't be reinitialize just because the window was
> resized (I hope I understood the code right that you actually do that).
> E.g. with -vo gl you can resize the window while the video is paused and
> it will immediately display the correctly scaled image.
>

http://www.eggheadcafe.com/software/aspnet/31258456/how-to-change-backbuffer.aspx

The problem is that on window resize, the old backbuffer dimensions
remain. I should always have backbuffer's width/height identical to
current window's size. It doesn't get updated by itself. So - I should
call IDirect3D9Device::Reset() on resize and enter new PRESENT_PARAMS.
The only thing I can keep is the OffscreenSurface, because it's
dimensions are always the same (movie source dimensions).  The problem
- Reset() doesn't succeed in case you don't free all D3D related
resources, which includes OffscreenSurface and all this equals to
destroying the surface and D3D Device and recreate them again. You
don't shut down the whole D3D actually, take a look in D3DUninit:

    /* Destroy D3D Context inside the window. */
    D3DDestroyContext();

    /* Stop the whole D3D. */
    if (NULL != gpD3DHandle)
    {
        mp_msg(MSGT_VO,MSGL_V,"<vo_direct3d>Calling Direct3D9_Release\r\n");
        IDirect3D9_Release (gpD3DHandle);
    }

Shutting down the whole D3D requires IDirect3D9_Release as well and I
don't do it in D3DDestroyContext. On my computer this resize operation
(including offscreen/adapter release) is fast... Nevertheless I don't
see how to make it better, this is API limitation IMHO. Any ideas?

>> +    /* Copy U. Casting to char * is here to suppress a compiler warning. */
>> +    Dst = (char *)stLockedRect.pBits + stLockedRect.Pitch * gSrcHeight
>> +          + UVstride * y + x;
>
> Personally I wouldn't comment on that cast, but just to make it clear:
> stLockedRect.pBits is "void *" and doing pointer arithmetic with that is
> _wrong_, so the cast is necessary to even have valid C (I still consider
> it bad that the compiler only warns instead of refusing to compile it).
>

Ok, how should I proceed? Something like that:

    /* Copy U */
    Dst = stLockedRect.pBits;
    Dst = Dst + stLockedRect.Pitch * gSrcHeight

I've done it that way and I hope that's correct.

>> +w32_common author: Reimar Doeffinger <reimar.doeffinger at stud.uni-karlsruhe.de>
>
> I am not the author, just the maintainer.
>
>> +Known libvo drivers which use this code: vo_gl.c, vo_direct3d.c
>
> vo_gl2.c, too.
>

The documentation from this file was integrated as Doxygen
documentation inside w32_common.c. Diego wanted this.