Re: [Live-devel] JPEG Video RTP Source ignores quantization table precision

[Joshua Hitchen]

Ahh yes, sorry, I should have paid more attention when writing that. Honestly 
it was a suggestion as a temporary patch till I get the time to write a 16 bit 
table support patch, which is why I rushed it.
The numtables variable was for future support (as was the way of calculating 
the increase to curLen).
The off-by-one error in the for loop was just negligence, but is an easy fix, 
the same goes for the incorrect check on the if statement.
The reason that I use the for loop rather than discarding if the entire 
precision field is not 0 is that there is the possibility of the excess bits 
not being zero - this is why the RFC states that excess bits *must* be ignored.

Other than that I fully understand your reasoning for keeping it as-is, 
although some documentation of the lack of support could be a good idea.


Regards

Josh Hitchen

-Original Message-
From: live-devel <live-devel-boun...@ns.live555.com> On Behalf Of Ross Finlayson
Sent: 09 March 2018 16:08
To: LIVE555 Streaming Media - development & use <live-de...@ns.live555.com>
Subject: Re: [Live-devel] JPEG Video RTP Source ignores quantization table 
precision

> 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

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

Re: [Live-devel] JPEG Video RTP Source ignores quantization table precision

[Ross Finlayson]

> 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

Re: [Live-devel] JPEG Video RTP Source ignores quantization table precision

[Joshua Hitchen]

> This may (perhaps) have been an oversight.  The “JPEGVideoRTPSource” code 
> basically just implements the code that’s in Appendix B of RFC 2435 (which 
> defines the RTP payload format for JPEG video).  I.e., it
> transforms the information that was in the special RTP JPEG header (and 
> quantization tables, if present) into a corresponding JPEG image header.  
> It’s possible that this code is incomplete.

Appendix B does specifically state that the code there is only for 8 bit 
precision quantization tables, but here is a code snippet for recognising the 
presence of double precision tables:

(first uncomment the declaration of precision at line 384, then insert this 
after the declaration of length on the next line)

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.
}


Regards

Josh Hitchen
Development

Metro Security (GB) PLC
www.metrosecurity.co.uk

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

Re: [Live-devel] JPEG Video RTP Source ignores quantization table precision

[Ross Finlayson]

> On Feb 17, 2018, at 4:13 AM, Joshua Hitchen  wrote:
> 
> Hi,
> Sorry to bother you on something potentially purposeful – it just seems like 
> you may have meant to temporarily remove this functionality given that some 
> of the code for it exists but is commented out.
> In JPEGVideoRTPSource.cpp you set the precision in the DQT segments to 0 
> (meaning 8 bit), even though the corresponding sink supports setting the 
> precision via a call to the quantizationTables method.

This may (perhaps) have been an oversight.  The “JPEGVideoRTPSource” code 
basically just implements the code that’s in Appendix B of RFC 2435 (which 
defines the RTP payload format for JPEG video).  I.e., it transforms the 
information that was in the special RTP JPEG header (and quantization tables, 
if present) into a corresponding JPEG image header.  It’s possible that this 
code is incomplete.

If you’re sure about this, feel free to propose a patch to 
“JPEGVideoRTPSource.cpp"


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