Den 2009-06-16 13:58 skrev Pierre Ossman:
> On Thu, 11 Jun 2009 13:30:41 +0200
> Peter Rosin <p...@lysator.liu.se> wrote:
> 
>> +The least significant four bits of *compression-control* byte inform
>> +the client which zlib compression streams should be reset before
>> +decoding the rectangle. Each bit is independent and corresponds to a
>> +separate zlib stream that should be reset:
>> +
>> +================== ====================================================
>> +Bit                Description
>> +================== ====================================================
>> +0                  if 1: reset stream 0
>> +1                  if 1: reset stream 1
>> +2                  if 1: reset stream 2
>> +3                  if 1: reset stream 3
>> +================== ====================================================
> 
> I think the "if 1:" part is implied. :)

Me too, but I wanted to first get the spec as written by Const (or
whoever wrote it) into the document using as little modifications
as possible, strange footnotes, missing references and all. We can
improve the text in later commits.

>> +One of three possible compression methods are supported in the Tight
>> +encoding. These are **BasicCompression**, **FillCompression** and
>> +**JpegCompression**. If the bit 7 (the most significant bit) of
>> +*compression-control* byte is 0, then the compression type is
>> +**BasicCompression**. In that case, bits 7-4 (the most significant four
>> +bits) of *compression-control* should be interpreted as follows:
>> +
>> +=============== =================== ===================================
>> +Bits            Binary value        Description
>> +=============== =================== ===================================
>> +5-4             00                  *UseStream0*
>> +..              01                  *UseStream1*
>> +..              10                  *UseStream2*
>> +..              11                  *UseStream3*
>> +6               0                   ---
>> +..              1                   *ReadFilterId*
>> +7               0                   **BasicCompression**
>> +=============== =================== ===================================
>> +
> 
> ReadFilterId is never explained.

It is explaned how it works though, first parapraph below
**BasicCompression**. But I agree that this is not enough.

> And the rest of the document uses lower case and dashes for keywords.

Not consistently, but I agree.

>> +Otherwise, if the bit 7 of *compression-control* is set to 1, then the
>> +compression method is either **FillCompression** or
>> +**JpegCompression**, depending on other bits of the same byte:
>> +
>> +=============== =================== ===================================
>> +Bits            Binary value        Description
>> +=============== =================== ===================================
>> +7-4             1000                **FillCompression**
>> +..              1001                **JpegCompression**
>> +..              any other           invalid
>> +=============== =================== ===================================
>> +
>> +Note: **JpegCompression** may only be used when *bits-per-pixel* is
>> +either 16 or 32.
> 
> Is this really a requirement, or just how the servers are implemented?
> I.e. will the clients cope with that method when bpp is 8?

Don't think so. My implementation will not.

> We should also mention that the server must not use JPEG when no
> quality level has been selected.
> 
> (I know this is mentioned in your next patch, but it should be pointed
> out here as well for clarity)

Right. I agree.

>> +**FillCompression**
>> +
>> +    If the compression type is "fill", then the only pixel value
>> +    follows, in client pixel format (see [NOTE1]_). This value applies
>> +    to all pixels of the rectangle.
> 
> These tight specific pixels are used everywhere. Perhaps they should be
> mentioned early, like CPIXEL for ZRLE?
> 
> And when you mention "fill" like that, it seems like you're referring
> to some canonical name. I suggest to just leave those kinds of short
> names out and just the **FillCompression** type of names.
> 
>> +**JpegCompression**
>> +
>> +    If the compression type is "jpeg", the following data stream looks
>> +    like this:
>> +
>> +    =============== =================== ===============================
>> +    No. of bytes    Type                Description
>> +    =============== =================== ===============================
>> +    1-3                                 *length* in compact
>> +                                        representation
>> +    *length*        ``U8`` array        *jpegData*
>> +    =============== =================== ===============================
>> +
>> +    *length* is compactly represented in one, two or three bytes,
>> +    according to the following scheme:
>> +
>> +    =========================== =======================================
>> +    Value                       Description
>> +    =========================== =======================================
>> +    0xxxxxxx                    for values 0..127
>> +    1xxxxxxx 0yyyyyyy           for values 128..16383
>> +    1xxxxxxx 1yyyyyyy zzzzzzzz  for values 16384..4194303
>> +    =========================== =======================================
>> +
>> +    Here each character denotes one bit, xxxxxxx are the least
>> +    significant 7 bits of the value (bits 0-6), yyyyyyy are bits 7-13,
>> +    and zzzzzzzz are the most significant 8 bits (bits 14-21). For
>> +    example, decimal value 10000 should be represented as two bytes:
>> +    binary 10010000 01001110, or hexadecimal 90 4E.
>> +
> 
> I suspect the Tight project never properly specified this, but what
> restrictions are placed on the JPEG data?
> 
> We should at the very least mention that it is a standard JFIF data
> stream.
> 
> And if we cannot properly specify the JPEG restrictions, we should
> mention that the area is a bit grey.

I agree.

>> +..  [NOTE3] The "gradient" filter and "jpeg" compression may be used
>> +    only when bits-per-pixel value is either 16 or 32, not 8.
> 
> Again, is this a hard requirement?

Yes, think so.

>> +
>> +..  [NOTE4] The width of any Tight-encoded rectangle cannot exceed
>> +    2048 pixels. If a rectangle is wider, it must be split into several
>> +    rectangles and each one should be encoded separately.
> 
> Dito.

My implementation does not have this limitation. Seemed like a silly thing
to explicitly check for in the client :-)
But my server does not generate such rects, because of this wording.

> Also, using footnotes without any reference in the text seems wrong.

Agreed.

So, again, I think all these deficiencies should be fixed in a followup
commit (or commits)...

Cheers,
Peter


------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensing option that enables unlimited
royalty-free distribution of the report engine for externally facing 
server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
tigervnc-rfbproto mailing list
tigervnc-rfbproto@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tigervnc-rfbproto

Reply via email to