Re: [PATCH v2] tinyjpeg: Dynamic luminance quantization table for Pixart JPEG
Hi, On 04/25/2012 06:09 PM, Jean-Francois Moine wrote: Hi Hans, snip BTW, I don't think the exposure and gain controls use the right registers as they are coded in the actual gspca pac7302 subdriver. The ms-windows driver uses the registers (3-80 / 3-03), (3-05 / 3-04), 3-03, 3-04 and 3-05 are already known and they all influence framerate / exposure in some way. I've also ran some tests with 3-80, again it influences framerate in some way (*). We already have a well tested and working, fine-grained way to control exposure so I think it is best to leave things as is exposure wise. (3-12) 3-12 is interesting, it is a new gain control. The pull request I've just send (with you in the CC) contains a patch to improve gain control using both 3-10 and 3-12 together. and (1-80) 1-80 is compression balance, since our decompression code for higher compression settings (markers 68) still is not perfect this is best left untouched. *) Note I've documented all registers I've ran tests with as part of the patchset for which I've just send a pull request. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tinyjpeg: Dynamic luminance quantization table for Pixart JPEG
Hi, On 04/25/2012 06:09 PM, Jean-Francois Moine wrote: Hi Hans, On Wed, 25 Apr 2012 16:19:57 +0200 Hans de Goedehdego...@redhat.com wrote: You say that the marker cannot be in the range 0..31 (index 0..7), but I have never seen a value lower than 68 (index 17). If you change register 0x80 in bank/page 1 to 42 on pac7311 or larger then circa 100 on pac7302, you will get markers with bit 8 set. When this happens you will initially get markers 0xa0 - 0xa4 ... 0xbc and the stream tends to stabilize on 0xbc. Likewise if you remove the artificial limiting of the pac7302 to 15 fps from the driver you will get markers 0x44 - 0x48 ... 0x7c. The images look a lot better with bit 8 set, so I plan to run some tests wrt what framerates can safely handle that (it uses more bandwidth) and set bit 8 on lower framerates. I carefully looked at the ms-windows pac7302 traces I have. The register 1-80 stays always in the range 0d..11, except sometimes 19 at start time. Right, that can mean one of 2 things: 1) The traces were made during daylight, so low exposure / high framerate, and enabling the lower compression modes (which cause bit 7 of the marker to get set) is a bad idea at high framerates 2) The windows driver never enables the low compression mode. I seriously doubt that this is the case, ie older versions of the pac7311 driver have (commented) writes to page 1 register 80 with high enough values to enable it and I'm pretty sure those writes come from windows traces. In these traces, the images with marker 44 (dec 68) look really better with all 08's as the quantization table. After having played with the quantization tables you've found I agree. [snip] Yeah short of someone disassembling and reverse-engineering the windows driver we will probably never figure out the exact correct tables. Well, I got the SPC230NC.SYS of the ms-windows pac7302 driver, but it is not easy to disassemble because it has no symbol table. But, inside, I found this tables just before the Huffman table: - 0006C888 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006C908 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006C988 08 08 08 08 08 08 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 - 0006CA08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 20 - 0006CA88 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006CB08 08 0b 0b 0b 0b 0b 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 20 20 20 20 20 20 20 40 40 40 40 40 40 40 40 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006CB88 11 12 12 18 15 18 2f 1a 1a 2f 63 42 38 42 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006CC08 10 0b 0c 0e 0c 0b 10 0e 0d 0e 12 11 10 13 18 28 1a 18 16 16 18 31 23 25 1d 28 3a 33 3d 3c 39 33 38 37 40 48 5c 4e 40 44 57 45 37 38 50 6D 51 57 5F 62 67 68 67 3E 4D 71 78 70 64 78 5C 65 67 63 Don't they look like quantization tables? Yes they do, good find! I've done yet more testing / trial and error with these tables and I've just pushed another Pixart JPEG patch to v4l-utils git switching to these new tables. Thanks! Also with these tables the quality difference between high/low compression mode becomes significantly less. So much less that I've decided to not further pursue enabling low compression mode in the gspca drivers, esp. since this will cause pain for people with an older libv4l. BTW, I don't think the exposure and gain controls use the right registers as they are coded in the actual gspca pac7302 subdriver. The ms-windows driver uses the registers (3-80 / 3-03), (3-05 / 3-04), (3-12) and (1-80) for autogain/exposure. The gspca test tarball of my web site includes a new AGC using these registers, but it does not work well. Maybe you could tell me what is wrong with it... Let me get back on that in a separate mail. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to
Re: [PATCH v2] tinyjpeg: Dynamic luminance quantization table for Pixart JPEG
Hi,
On 04/24/2012 12:34 PM, Jean-Francois Moine wrote:
On Mon, 23 Apr 2012 23:34:05 +0200
Hans de Goedehdego...@redhat.com wrote:
Thanks for your work on this! I've just spend almost 4 days wrestling
which the Pixart JPEG decompression code to try to better understand
these cams, and I have learned quite a bit and eventually came up
with a different approach.
But your effort is appreciated! After spending so much time on this
myself, I can imagine that it took you quite some time to come up
with your solution.
Attach is a 4 patch patchset which I plan to push to v4l-utils
tomorrow (after running some more tests in daylight). I'll also try
to do some kernel patches tomorrow to match...
Hi Hans,
I tried your patch, but I am not happy with the images I have (pac7302).
You say that the marker cannot be in the range 0..31 (index 0..7), but
I have never seen a value lower than 68 (index 17).
If you change register 0x80 in bank/page 1 to 42 on pac7311 or larger then
circa 100 on pac7302, you will get markers with bit 8 set. When this happens
you will initially get markers 0xa0 - 0xa4 ... 0xbc and the stream tends to
stabilize on 0xbc. Likewise if you remove the artificial limiting of
the pac7302 to 15 fps from the driver you will get markers 0x44 - 0x48 ...
0x7c.
The images look a lot better with bit 8 set, so I plan to run some tests
wrt what framerates can safely handle that (it uses more bandwidth) and set
bit 8 on lower framerates.
This last marker value (68) is the default when the images have no big
contrasts. With such images / blocks, the standard JPEG quantization
table does not work well. It seems that, for this value, the table
should be full of either 7 or 8 (8 gives a higher contrast).
Using a table with all 7's or 8's looses a lot of sharpness in image which
high frequency components, for example the grain of a rough wall completely
disappears.
I've (once more) tried to use a more simplified / flat quant table, I
ended up with the table below, so as to not loose too much sharpness:
0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x0b, 0x10, 0x10,
0x10, 0x10, 0x10, 0x10, 0x10, 0x10, 0x10, 0x20,
0x20, 0x20, 0x20, 0x20, 0x20,
0x28, 0x28, 0x28,
0x28, 0x28, 0x28, 0x28,
0x30, 0x30, 0x30, 0x30,
0x30, 0x30, 0x30, 0x30,
0x38, 0x38, 0x38, 0x38,
0x38, 0x38, 0x38,
0x40, 0x40, 0x40, 0x40, 0x40,
0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40,
0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40, 0x40,
Using the same qfactor as before, so the 0x0b translates to 7, with this
table the images only change slightly, mostly because of the lower
values at the end. This means we loose some sharpness of the picture
(loose of high frequency components), and when watching a moving picture
with that loose of sharpness we also loose some noise. But other then that
the results are almost the same.
When using 0x08 rather then 0x07 we seem to get accumlating DC errors,
leading to clear block divisions at the end of an mcu row.
Given the above I've decided to stick with my solution for now, I know
it is not the ideal solution, but I believe it is the best we have. Also
notice that my solution in essence gives us the same table for marker 68
as we used before your tinyjpeg: Better luminance quantization table for
Pixart JPEG patch.
Here is the sequence which works better (around line 1420 of tinyjpeg.c):
-8--
/* And another special Pixart feature, the DC quantization
factor is fixed! */
qt[0] = 7; // 8 gives a higher contrast
// special case for 68
if (marker == 68) {
for (i = 1; i 64; i++)
qt[i] = 7; // also works with 8
} else {
for (i = 1; i 64; i++) {
j = (standard_quantization[0][i] * comp + 50) / 100;
qt[i] = (j 255) ? j : 255;
}
}
build_quantization_table(priv-Q_tables[0], qt);
-8--
About the other marker values, it seems also that the quantization
tables are not optimal: some blocks are either too much (small
contrasted lines) or not enough (big pixels) decompressed. As you know,
a finer adjustment would ask for a long test time, so, I think we can
live with your code.
Yeah short of someone disassembling and reverse-engineering the windows driver
we will probably never figure out the exact correct tables.
Regards,
Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tinyjpeg: Dynamic luminance quantization table for Pixart JPEG
Hi Hans, On Wed, 25 Apr 2012 16:19:57 +0200 Hans de Goede hdego...@redhat.com wrote: You say that the marker cannot be in the range 0..31 (index 0..7), but I have never seen a value lower than 68 (index 17). If you change register 0x80 in bank/page 1 to 42 on pac7311 or larger then circa 100 on pac7302, you will get markers with bit 8 set. When this happens you will initially get markers 0xa0 - 0xa4 ... 0xbc and the stream tends to stabilize on 0xbc. Likewise if you remove the artificial limiting of the pac7302 to 15 fps from the driver you will get markers 0x44 - 0x48 ... 0x7c. The images look a lot better with bit 8 set, so I plan to run some tests wrt what framerates can safely handle that (it uses more bandwidth) and set bit 8 on lower framerates. I carefully looked at the ms-windows pac7302 traces I have. The register 1-80 stays always in the range 0d..11, except sometimes 19 at start time. In these traces, the images with marker 44 (dec 68) look really better with all 08's as the quantization table. [snip] Yeah short of someone disassembling and reverse-engineering the windows driver we will probably never figure out the exact correct tables. Well, I got the SPC230NC.SYS of the ms-windows pac7302 driver, but it is not easy to disassemble because it has no symbol table. But, inside, I found this tables just before the Huffman table: - 0006C888 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006C908 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006C988 08 08 08 08 08 08 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 40 - 0006CA08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 08 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 20 - 0006CA88 10 10 10 10 10 10 20 20 20 20 20 20 20 20 20 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006CB08 08 0b 0b 0b 0b 0b 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 20 20 20 20 20 20 20 40 40 40 40 40 40 40 40 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006CB88 11 12 12 18 15 18 2f 1a 1a 2f 63 42 38 42 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 63 - 0006CC08 10 0b 0c 0e 0c 0b 10 0e 0d 0e 12 11 10 13 18 28 1a 18 16 16 18 31 23 25 1d 28 3a 33 3d 3c 39 33 38 37 40 48 5c 4e 40 44 57 45 37 38 50 6D 51 57 5F 62 67 68 67 3E 4D 71 78 70 64 78 5C 65 67 63 Don't they look like quantization tables? (the table 0006CB08 is quite the same the flat table you tried!) BTW, I don't think the exposure and gain controls use the right registers as they are coded in the actual gspca pac7302 subdriver. The ms-windows driver uses the registers (3-80 / 3-03), (3-05 / 3-04), (3-12) and (1-80) for autogain/exposure. The gspca test tarball of my web site includes a new AGC using these registers, but it does not work well. Maybe you could tell me what is wrong with it... Regards. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] tinyjpeg: Dynamic luminance quantization table for Pixart JPEG
On Mon, 23 Apr 2012 23:34:05 +0200
Hans de Goede hdego...@redhat.com wrote:
Thanks for your work on this! I've just spend almost 4 days wrestling
which the Pixart JPEG decompression code to try to better understand
these cams, and I have learned quite a bit and eventually came up
with a different approach.
But your effort is appreciated! After spending so much time on this
myself, I can imagine that it took you quite some time to come up
with your solution.
Attach is a 4 patch patchset which I plan to push to v4l-utils
tomorrow (after running some more tests in daylight). I'll also try
to do some kernel patches tomorrow to match...
Hi Hans,
I tried your patch, but I am not happy with the images I have (pac7302).
You say that the marker cannot be in the range 0..31 (index 0..7), but
I have never seen a value lower than 68 (index 17).
This last marker value (68) is the default when the images have no big
contrasts. With such images / blocks, the standard JPEG quantization
table does not work well. It seems that, for this value, the table
should be full of either 7 or 8 (8 gives a higher contrast).
Here is the sequence which works better (around line 1420 of tinyjpeg.c):
-8--
/* And another special Pixart feature, the DC quantization
factor is fixed! */
qt[0] = 7; // 8 gives a higher contrast
// special case for 68
if (marker == 68) {
for (i = 1; i 64; i++)
qt[i] = 7; // also works with 8
} else {
for (i = 1; i 64; i++) {
j = (standard_quantization[0][i] * comp + 50) / 100;
qt[i] = (j 255) ? j : 255;
}
}
build_quantization_table(priv-Q_tables[0], qt);
-8--
About the other marker values, it seems also that the quantization
tables are not optimal: some blocks are either too much (small
contrasted lines) or not enough (big pixels) decompressed. As you know,
a finer adjustment would ask for a long test time, so, I think we can
live with your code.
Best regards.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] tinyjpeg: Dynamic luminance quantization table for Pixart JPEG
In PJPG blocks, a marker gives the quantization tables to use for image
decoding. This patch dynamically updates the luminance table when the
marker changes.
Note that the values of this table have been guessed from a small
number of images and that they may not work fine in some situations,
but, in most cases, the images are better.
Signed-off-by: Jean-François Moine moin...@free.fr
diff --git a/lib/libv4lconvert/tinyjpeg-internal.h
b/lib/libv4lconvert/tinyjpeg-internal.h
index 702a2a2..4041251 100644
--- a/lib/libv4lconvert/tinyjpeg-internal.h
+++ b/lib/libv4lconvert/tinyjpeg-internal.h
@@ -103,6 +103,7 @@ struct jdec_private {
#if SANITY_CHECK
unsigned int current_cid; /* For planar JPEG */
#endif
+ unsigned char marker; /* for PJPG (Pixart JPEG) */
/* Temp space used after the IDCT to store each components */
uint8_t Y[64 * 4], Cr[64], Cb[64];
diff --git a/lib/libv4lconvert/tinyjpeg.c b/lib/libv4lconvert/tinyjpeg.c
index 2c2d4af..d986a45 100644
--- a/lib/libv4lconvert/tinyjpeg.c
+++ b/lib/libv4lconvert/tinyjpeg.c
@@ -1376,6 +1376,8 @@ static void decode_MCU_2x1_3planes(struct jdec_private
*priv)
IDCT(priv-component_infos[cCr], priv-Cr, 8);
}
+static void build_quantization_table(float *qtable, const unsigned char
*ref_table);
+
static void pixart_decode_MCU_2x1_3planes(struct jdec_private *priv)
{
unsigned char marker;
@@ -1384,10 +1386,8 @@ static void pixart_decode_MCU_2x1_3planes(struct
jdec_private *priv)
/* I think the marker indicates which quantization table to use, iow
a Pixart JPEG may have a different quantization table per MCU, most
MCU's have 0x44 as marker for which our special Pixart quantization
- tables are correct. Unfortunately with a 7302 some blocks also have
0x48,
- and sometimes even other values. As 0x48 is quite common too, we
really
- need to find out the correct table for that, as currently the blocks
- with a 0x48 marker look wrong. During normal operation the marker
stays
+ tables are correct. [jfm: snip]
+ During normal operation the marker stays
within the range below, if it gets out of this range we're most
likely
decoding garbage */
if (marker 0x20 || marker 0x7f) {
@@ -1396,6 +1396,53 @@ static void pixart_decode_MCU_2x1_3planes(struct
jdec_private *priv)
(unsigned int)marker);
longjmp(priv-jump_state, -EIO);
}
+
+ /* rebuild the Y quantization table when the marker changes */
+ if (marker != priv-marker) {
+ unsigned char quant_new[64];
+ int i, j;
+ /*
+* table to rebuild the Y quantization table
+* index 1 = marker / 4
+* index 2 = 4 end indexes in the quantization table
+* values = 0x08, 0x10, 0x20, 0x40, 0x63
+* jfm: The values have been guessed from 4 images, so,
+* better values may be found...
+*/
+ static const unsigned char q_tb[12][4] = {
+ { 64, 64, 64, 64 }, /* 68 */
+ { 8, 32, 64, 64 },
+ { 1, 16, 50, 64 },
+ { 1, 16, 30, 60 }, /* 80 */
+ { 1, 8, 16, 32 },
+ { 1, 4, 16, 31 },
+ { 1, 3, 16, 30 },
+ { 1, 2, 16, 21 },
+ { 1, 1, 16, 18 }, /* 100 */
+ { 1, 1, 16, 17 },
+ { 1, 1, 16, 16 },
+ { 1, 1, 15, 15 },
+ };
+
+ priv-marker = marker;
+ j = marker - 68;
+ if (j 0)
+ j = 0;
+ j = 2;
+ if (j sizeof q_tb / sizeof q_tb[0])
+ j = sizeof q_tb / sizeof q_tb[0] - 1;
+ for (i = 0; i q_tb[j][0]; i++)
+ quant_new[i] = 0x08;
+ for (; i q_tb[j][1]; i++)
+ quant_new[i] = 0x10;
+ for (; i q_tb[j][2]; i++)
+ quant_new[i] = 0x20;
+ for (; i q_tb[j][3]; i++)
+ quant_new[i] = 0x40;
+ for (; i 64; i++)
+ quant_new[i] = 0x63;
+ build_quantization_table(priv-Q_tables[0], quant_new);
+ }
skip_nbits(priv-reservoir, priv-nbits_in_reservoir, priv-stream, 8);
// Y
@@ -1948,6 +1995,7 @@ static int parse_JFIF(struct jdec_private *priv, const
unsigned char *stream)
if (!priv-default_huffman_table_initialized) {
build_quantization_table(priv-Q_tables[0],
pixart_quantization[0]);
