Hi Armin, 2016-05-19 9:18 GMT+02:00 Armin Novak <armin.no...@thincast.com>:
> Hey Ondrej, > > sorry for the late reply, did indeed missed to reply. > neverminds, thanks for the replay. > Most of the code works on a big endian, because > > Stream_Read_/Stream_Write_ macros are used to read/write data from/to > > server. There is a problem if Stream_Pointer is used to read raw data, > > because the data are always stored as little endian. It is problem > > especially with wide strings and colors. It seems to me that usage of > > something like Stream_Read_Bitmap/Stream_Read_String would be > > suboptimal, because it requires additional data processing, but maybe > > this is the only right way to do it. > Wide String read / write is using internal conversion to UTF-8, so the > use of raw data should not be to big an issue there. For color > conversion stuff the data layout for data from RDP is fixed in the spec, > so the layout will not change there. Converting bitmaps from local to > RDP is a different beast though, one I still try to fix properly. > Did not know that this is fixed in the documentation, thanks for notice, have to take a look... > > Some of the color conversions access the data byte per byte, some of > > them access them as UINT16, or UINT32 currently. This is obviously a > > problem for a big endian. It seems that your patches fixes this > > problem thanks to ReadColor/WriteColor functions, so it is great! > > Unfortunately there is another problem. X11 client requires "inverted" > > colors on a big endian (not sure about another clients). I realized > > that "inverted" color for ARGB32 should be BGRA (not ABGR as it is > > used currently), however this doesn't work for another bit depths. > > "Inverted" color for RGB16 is not BGR16, but RGB16 with swapped bytes, > > i.e. loaded as little endian and stored as big endian. > > > > So, I am not sure whether BGR color formats are really needed at all. > > Don't you know whether some of the clients really need BGR? It seems > > to me that BGR color formats were intended to handle colors on a big > > endian, but it was probably misunderstanding. So, maybe the "inverted" > > color formats (i.e. BGR, ABGR, XBGR) may be removed and something like > > FREERDP_PIXEL_FORMAT_INVERT may be introduced instead. What do you > > think about it? > The formats are necessary as some data in RDP is sent as BGR(A) data and > I try to avoid the invert mess that freerdp color conversion currently > is. Actually I want to keep such external stuff out of the library > altogether. Just interpret color as a byte array so that conversion is > always done right. Let the client code handle the decision for the > correct destination format (a switch to select that on different > conditions for gdi_init and you are ready to go) > So this way you just check which format is used by the client to > represent data (no 16/32 bit destinction necessary, just the correct > local framebuffer format) and every other conversion is already done > internally. > Ok, then the following code from xf_client.c is totally wrong... xfc->invert = (ImageByteOrder(xfc->display) == MSBFirst) ? TRUE : FALSE; ... xfc->format = (xfc->invert) ? PIXEL_FORMAT_RGBA32 : PIXEL_FORMAT_BGRA32; and something like FREERDP_PIXEL_FORMAT_ENDIANNESS should be added instead, what do you think about it? -- Regards Ondrej ------------------------------------------------------------------------------ Mobile security can be enabling, not merely restricting. Employees who bring their own devices (BYOD) to work are irked by the imposition of MDM restrictions. Mobile Device Manager Plus allows you to control only the apps on BYO-devices by containerizing them, leaving personal data untouched! https://ad.doubleclick.net/ddm/clk/304595813;131938128;j _______________________________________________ FreeRDP-devel mailing list FreeRDP-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/freerdp-devel
