Using emit_byte for every single byte kills performance. It eliminates much of the advantage of using the accelerated Huffman codec.
However, I checked in a fix which should make things backward compatible again while preserving performance. Since I can't yet build Xvnc, I have no way of verifying this fix at the high level, but it seems to work at the low level. The crux of the problem was that, in order to avoid checking the buffer size every time a byte was written (which is very slow), I had to modify encode_one_block() such that it pre-emptively cleared enough space in the buffer before writing anything. This meant that sometimes empty_output_buffer() would be called when the buffer was not 100% full. I modified the default empty_output_buffer() in jdatadst.c to handle this case, but the implementation of empty_output_buffer() in TigerVNC did not handle the case. Ultimately, I want to modify TigerVNC such that it does not use buffered JPEG compression at all (but rather compresses straight from the Xvnc framebuffer, like TurboVNC does.) That would have rendered this issue moot. However, I agree that libjpeg/SIMD needs to be backward compatible, as long as making it so does not compromise performance. Thus, I came up with a solution which (I think) achieves both ends. The solution uses a small local buffer in encode_one_block() to write the Huffman output if that function detects that the main output buffer is almost full. This eliminates the need for any special handling in empty_output_buffer() and should restore 100% backward compatibility. Pierre Ossman wrote: > I found the problem, and it was basically the same as for cjpeg. > > The empty_output_buffer() callback is part of the API, and when you > forced it to check free_in_buffer, you were no longer backwards > compatible and the tight encoder of course broke down. I don't > understand how this bug wasn't present when Adam tested. > > Anyway, I think that we should retain libjpeg compatibility. As your > new huffman encoder outputs several bytes at once, that might mean a > performance hit though. > > Could you see how much damage it does to replace all *buffer++ = with > something like the previous emit_byte()? > > Rgds > ------------------------------------------------------------------------------ Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are powering Web 2.0 with engaging, cross-platform capabilities. Quickly and easily build your RIAs with Flex Builder, the Eclipse(TM)based development software that enables intelligent coding and step-through debugging. Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com _______________________________________________ Tigervnc-devel mailing list Tigervnc-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tigervnc-devel
