Re: [PATCH 3/5] amstream: Reject incompatible media types in AMDirectDrawStream::ReceiveConnection.

[Zebediah Figura]

On 9/5/20 1:05 PM, Anton Baskanov wrote:
> On Sunday, 30 August 2020 03:37:16 +07 you wrote:
> > On 8/29/20 8:51 AM, Anton Baskanov wrote:
> > > Signed-off-by: Anton Baskanov
> > >
> > > dlls/amstream/ddrawstream.c    |  41 +++++++++++-
> > >  dlls/amstream/tests/amstream.c | 110 +++++++++++++++++++++++++++++++++
> > >  2 files changed, 150 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/dlls/amstream/ddrawstream.c b/dlls/amstream/ddrawstream.c
> > > index a713b80be45..dcb36d0028f 100644
> > > --- a/dlls/amstream/ddrawstream.c
> > > +++ b/dlls/amstream/ddrawstream.c
> > > @@ -53,6 +53,44 @@ struct ddraw_stream
> > >
> > > static HRESULT ddrawstreamsample_create(struct ddraw_stream *parent,
> > >  IDirectDrawSurface *surface,>  
> > >      const RECT *rect, IDirectDrawStreamSample **ddraw_stream_sample);
> > >
> > > +static BOOL is_media_type_compatible(const AM_MEDIA_TYPE *media_type,
> > > const DDSURFACEDESC *format) +{
> > > - const VIDEOINFOHEADER *video_info = (const VIDEOINFOHEADER
> > >
> > > *)media_type->pbFormat; +
> > > - if ((format->dwFlags & DDSD_WIDTH) && video_info->bmiHeader.biWidth
> > >
> > > != format->dwWidth) +        return FALSE;
> > > -
> > > - if ((format->dwFlags & DDSD_HEIGHT) &&
> > >
> > > abs(video_info->bmiHeader.biHeight) != format->dwHeight) +        return
> > > FALSE;
> > > -
> > > - if (format->dwFlags & DDSD_PIXELFORMAT)
> > > - {
> > > -
> > >    const GUID *subtype = &GUID_NULL;
> > > -
> > >    switch (format->ddpfPixelFormat.u1.dwRGBBitCount)
> > > -
> > >    {
> > > -
> > >    case 8:
> > > -
> > >        subtype = &MEDIASUBTYPE_RGB8;
> > > -
> > >        break;
> > > -
> > >    case 16:
> > > -
> > >        if (format->ddpfPixelFormat.u3.dwGBitMask == 0x7e0)
> > > -
> > >            subtype = &MEDIASUBTYPE_RGB565;
> > > -
> > >        else
> > > -
> > >            subtype = &MEDIASUBTYPE_RGB555;
> > > -
> > >        break;
> > > -
> > >    case 24:
> > > -
> > >        subtype = &MEDIASUBTYPE_RGB24;
> > > -
> > >        break;
> > > -
> > >    case 32:
> > > -
> > >        subtype = &MEDIASUBTYPE_RGB32;
> > > -
> > >        break;
> > > -
> > >    }
> > > -
> > >    if (!IsEqualGUID(&media_type->subtype, subtype))
> > > -
> > >        return FALSE;
> > > - }
> >
> > I wonder if there's a nice way to combine the pixel format translation
> > with the validity check from 2/5, maybe along the lines of a helper
> > function that checks both.
>
> I've rearranged things so that the media type is now translated to pixel 
> format in ReceiveConnection and the compatibility check is done between two 
> pixel formats.

Yech, on rereading my review was more than a little confused. v2 seems
like an improvement regardless, though :-)

(What I kind of want to do is translate ddraw pixel formats back into
amstream subtypes and compare those directly, but given that there's no
other reason to do that translation, it's probably best the way you have
it...)