Hi Armin, I've tried your color_conversion_fix_v3 branch on Fedora 24 and it fails to me with the following ouput: [16:46:20:397] [6168:6169] [INFO][com.freerdp.gdi] - Local framebuffer format PIXEL_FORMAT_BGRX32 [16:46:20:397] [6168:6169] [INFO][com.freerdp.gdi] - Remote framebuffer format PIXEL_FORMAT_RGB16 [16:46:20:400] [6168:6169] [ERROR][com.freerdp.core.codecs] - Failed to create h264 codec context [16:46:20:400] [6168:6169] [ERROR][com.freerdp.gdi] - failed to initialize gdi [16:46:20:400] [6168:6169] [ERROR][com.freerdp.core] - freerdp_post_connect failed [16:46:20:400] [6168:6169] [ERROR][com.freerdp.core] - freerdp_set_last_error ERRCONNECT_POST_CONNECT_FAILED [0x20003] [16:46:20:400] [6168:6169] [ERROR][com.freerdp.client.x11] - Freerdp connect error exit status 1 Segmentation fault (core dumped)
Have you any idea what is wrong? 2016-04-29 11:57 GMT+02:00 Ondrej Holy <oh...@redhat.com>: > Hi Armin, > > I am not FreeRDP upstream maintainer, but I glad to see you are working on > color conversions overhaul. Current state with color conversions seems > pretty nonuniform and complicated. I am preparing fixes for big endian > architectures, because there are corrupted colors and several other > problems. I used following pull request for a discussion: > https://github.com/FreeRDP/FreeRDP/pull/3284 > > 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. > > 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? > > Btw it is not possible to build FreeRDP with unit tests > (-DBUILD_TESTING=ON), because the tests are broken by your patches. I > wanted to test your patches on a big endian also, but xfreerdp segfaulted. > I need to take a look why... > > 2016-04-25 9:26 GMT+02:00 Armin Novak <armin.no...@thincast.com>: > >> Hi there, >> >> I'm working on a overhaul of the software GDI color conversion. >> My current work is here: >> https://github.com/akallabeth/freerdp/tree/color_conversion_fix_v3 >> >> This pull requires refactoring of most of libfreerdp and there are still >> a few issues left (listing later on) >> but it is aimed at fixing the following issues: >> >> * Decouple local framebuffer format from remote one >> This will allow having a local 32bit framebuffer for a 8/15/16/24 bit >> RDP session >> >> * Allow arbitrary color ordering of local framebuffer and fix all >> RFX/H264/GDI functions to >> respect that color order >> >> * Allow externally supplied buffers to be used in GDI layer >> >> * Remove some performance bottlenecks (do less copying of input data), >> re-factor the touched API to fix some flaws. >> >> It is working quite well so far for RFX/GFX/H264 but there remain some >> problems with GDI mode. >> >> I'd like some people have a look at it before creating a pull for it, >> so if anyone is interested do have a look at the branch. >> >> best >> Armin >> >> >> ------------------------------------------------------------------------------ >> Find and fix application performance issues faster with Applications >> Manager >> Applications Manager provides deep performance insights into multiple >> tiers of >> your business applications. It resolves application problems quickly and >> reduces your MTTR. Get your free trial! >> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z >> _______________________________________________ >> FreeRDP-devel mailing list >> FreeRDP-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/freerdp-devel >> > > > > -- > Regards > > Ondrej > -- 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
