Created attachment 72373 [details] [review]
imlement colorkeying change

even better would be to detect the depth of the display and generate a tripplet based on the RGB masks. Also add a ckey = CLAMP (ckey, attr[i].min_value, attr[i].max_value);  to make sure it never sets something impossible.

Created attachment 72975 [details] [review]
better version of the patch

Not sure whats the best way to gte the depth and calculate the key from the masks.

would that be good?

        int ckey=0;

        switch(xcontext->depth) {
          case 15:
            ckey=(1<<10)|(2<<5)|3;
            break;
          case 16:
            ckey=(1<<11)|(2<<5)|3;
            break;
          case 24:
          case 32:
            ckey=(1<<16)|(2<<8)|3;
            break;
          default:
            GST_WARNING("unsupported color depth");
            break;
        }

I think that would be acceptable for now, the main problem is to make sure it does not set invalid values, the exact color can be anything and is irrelevant.

NACK to the patch in comment #4.  switching based on the display's depth is bogus, as it's perfectly common to have an RGB565 display that still expects a colour key in RGB888.  indeed, the only reason we never hit this is because everyone else on the planet always takes an RGB888 colour key.

the only sane way to do this is to key off max_value on the XV_COLORKEY attribute, which tells you exactly how many bits you have for the colour key.

(the one in comment #3 is perfectly reasonable.)

I've changed it back in cvs