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  On Behalf Of Ross Finlayson
Sent: 09 March 2018 16:08
To: LIVE555 Streaming Media - development & use 
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

[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

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

[Joshua Hitchen]

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.
Line 384 of JPEGVideoRTPSource.cpp is as follows:
// unsigned Precision = (unsigned)headerStart[resultSpecialHeaderSize + 1];
Is this a purposeful lack of support for the 16 bit quantization tables, or was 
it partially implemented then commented to prevent errors?
Should I throw away JPEG frames with 16 bit quantization tables in my 
JPEGVideoSource subclass?
If this was intentional would it not be better to remove support in the sink, 
or is that for compatibility with other receivers?

Regards

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

[Live-devel] Error in ByteStreamMultiFileSource.cpp

[Joshua Hitchen]

In the constructor for the ByteStreamMultiFileSource, to determine the number 
of sources you iterate fNumSources until the value pointed to by  
fileNameArray[fNumSources] is NULL, but there is no guarantee that the stored 
value after the pointer to the final file name pointer in the array will be 
null. This leads to access violations.

Here is my proposed method to replace the current implementation:
fNumSources = sizeof(fileNameArray);

Input File Names:
File Index 0: test 0.ext
File Index 1: test 1.ext
File Index 2: test 2.ext
File Index 3: test 3.ext
Actual number of files: 4
My Method result: 4
ByteStreamMultiFileSource.cpp Method's result: 9
Regards

Josh Hitchen


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

[Live-devel] Win32 config settings don't apply to Windows Audio Input Device makefile

[Joshua Hitchen]

When running genWindowsMakeFiles.cmd it does not apply the changes to the win32 
config file in the makefile for Windows Audio Input Device, it does properly 
apply the settings change to all other makefiles.
My changes:

  1.  Comment out "!include"
  2.  Set TOOLS32 to "C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC"
  3.  Change LINK_OPTS_0 to "$(linkdebug) msvcrt.lib"

Is this an error on my part or an issue with the live555 project? Of course, I 
can manually change the settings on the makefile but it'd be nice if that 
wasn't necessary.

Regards

Josh Hitchen


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