> unsigned curLen = 0;
> unsigned numtables = 0;
> for (int i = 8; i >= 0; i--) //each bit in the Precision field indicates 
> value of a table read from right to left; 0 for 8 bit, 1 for 16 bit (see RFC 
> 2435 section 3.1.8)
> {
>       if(curLen >= Length) break; //Ignore excess bits after all tables are 
> accounted for.
>       numtables++;
>       curLen += 64 * ((Precision & (1 << i - 1) + 1); //Set up for when 16 
> bit tables become supported. Equivalent to 64 * ((Precision & 2^(i-1)) + 1).
>       if((Precision & (1 << i - 1) == 0) return False; //Currently 
> unsupported - 16 bit precision table.
> }

Given that there seems to be so at least three things wrong with this:
        - i ranges from 8 through 0 (i.e., takes 9 different values)
        - The variable “numTables” is assigned but not used
        - You’re checking bits in the “Precision” field to be 0 to mean a 
16-bit precision table; you should instead be testing for 1-bits
I’m not going to be adding this to the code, thank you :-)

In any case, if we were ever to support 16-bit precisions in a quantization 
table, then we’d need to completely change the “createJPEGHeader()” function - 
to take the “Precision” field as parameter, and use it in the implementation.  
If and when someone proposes a patch to do that (not I, because I don’t think 
people should really be streaming JPEG anyway :-), then I’ll consider adding it 
to the code.  In the meantime, all we could really do is just check whether any 
16-bit tables are present, and, if so, reject the incoming RTP packet.  That’s 
what your code aims to do, but a much simpler way to do that would simply be to 
do
        if (Precision != 0) return False;
instead of your code.  But I’m not going to do that either, because it will 
cause a client who tries to receive/handle 16-bit tables to just fail silently. 
 Instead, the current code will cause a client who receives 16-bit tables to 
render an incorrect (i.e., bad-looking) JPEG image, which would better help 
them figure out what’s wrong.


Ross Finlayson
Live Networks, Inc.
http://www.live555.com/


_______________________________________________
live-devel mailing list
live-devel@lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel

Reply via email to