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

[Reimar Döffinger]

On Mon, Nov 17, 2008 at 11:41:04AM +0200, Georgi Petrov wrote:
> 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.

Hm. If you want to support it just using the real 3D API instead of
StretchRect might be nicer, except I guess that the 3D API would not
support YUV.
Anyway, you still should not be using your own aspect code, it e.g. does
not handle -monitor-aspect or even -monitor-pixel-aspect and probably
even more issues. Just leave out the three lines relating to panscan
and you should not have any issues (at least not more than with the
current code).

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

For dr you return an mp_image struct, and that does have a stride field.

> And why isn't DR turned on by
> default for drivers that support it?

Mostly bugs, but also because you can't really know if it will be
faster (and that is a fundamental problem, it can depend e.g. on
the memory access patterns of the decoder or video filter).
DR is actually quite useless most of the time.

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

No, no ideas, though it sounds like a very weird approach to let the
programmer handle the backbuffer themselves, sounds like useless
complexity to me (not that OpenGL doesn't have its share of mess).

> >> +    /* 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.

It was correct before, I was only complaining about the comment,
which gave the wrong impression IMO.
Feel free to write it in whichever way you prefer.

Greetings,
Reimar Doeffinger