[PATCH xserver] Xv: Use unsigned int for XvAdaptor type

[Olivier Fourdan]

Hi all,

Quick follow-up on this.

hw/xfree86/common/xf86xv.h defines the type as unsigned int:

typedef struct {
    unsigned int type;
    int flags;
    const char *name;
    ...
} XF86VideoAdaptorRec, *XF86VideoAdaptorPtr;

Whereas Xext/xvdix.h defines the type as unsigned char:

typedef struct {
    unsigned long base_id;
    unsigned char type;
    ...
} XvAdaptorRec, *XvAdaptorPtr;

And xf86XVInitAdaptors() does:

static Bool 
xf86XVInitAdaptors(ScreenPtr pScreen, XF86VideoAdaptorPtr * infoPtr, int number)
{
    XF86VideoAdaptorPtr adaptorPtr;
    ...
    XvAdaptorPtr pAdaptor, pa;
    ...
    for (pa = pAdaptor, na = 0, numAdaptor = 0; na < number; na++, adaptorPtr++) {
        adaptorPtr = infoPtr[na];

        ...
        pa->type = adaptorPtr->type;
        ...
    }
}

That would probably raise a compiler warning. But it doesn't really matter because constant values used in the bit field that may be bigger than a char are just used by the xf86 implemenation apparently, for things like:

        if (!(adaptorPtr->type & (XvPixmapMask | XvWindowMask)))
            continue;

So not actually used in XvAdaptor itself... So long story short, if we don't want to change the type in XvAdaptor, we should probably typecast in xf86XVInitAdaptors() and not use those values in Xwayland Xv implementation (which doesn't use xf86 Xv interface).

So that means 2 other patches instead of this one that we wouldn't want. Does that make sense?

Cheers,
Olivier

----- Original Message -----
> XF86VideoAdaptor uses an unsigned int for its type, and videoproto
> defines values greater than the size of a char for the bit mask used in
> the type field:
> 
>    #define XvPixmapMask     0x00010000
>    #define XvWindowMask     0x00020000
> 
> But the type used in XvAdaptor for the DIX is defined as an unsigned
> char, so the values set in videoproto won't fit in the XvAdaptor type
> field.
> 
> Beside, XFree86 Xv implementation copies the value from XF86VideoAdaptor
> (defined an unsigned int) to the XvAdaptor type (defined as an unsigned
> char), so the type for XvAdaptor type ought to be large enough.
> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  Note: This will break ABI with DDX, so we'd need to bump the ABI version
>  as well, but given we're in the middle of development phase, is it
>  necessary?
> 
>  Xext/xvdix.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Xext/xvdix.h b/Xext/xvdix.h
> index 621a1e3..59aaf3d 100644
> --- a/Xext/xvdix.h
> +++ b/Xext/xvdix.h
> @@ -144,7 +144,7 @@ typedef struct {
>  
>  typedef struct {
>      unsigned long base_id;
> -    unsigned char type;
> +    unsigned int type;
>      char *name;
>      int nEncodings;
>      XvEncodingPtr pEncodings;
> --
> 2.5.0
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel