1 files changed, 85 insertions, 26 deletions

diff --git a/src/lib/openjp2/mqc.c b/src/lib/openjp2/mqc.c
index 8a792b60..92a0d318 100644
--- a/src/lib/openjp2/mqc.c
+++ b/src/lib/openjp2/mqc.c
@@ -182,13 +182,10 @@ static opj_mqc_state_t mqc_states[47 * 2] = {
 
 static void opj_mqc_byteout(opj_mqc_t *mqc)
 {
-    /* avoid accessing uninitialized memory*/
-    if (mqc->bp == mqc->start - 1) {
-        mqc->bp++;
-        *mqc->bp = (OPJ_BYTE)(mqc->c >> 19);
-        mqc->c &= 0x7ffff;
-        mqc->ct = 8;
-    } else if (*mqc->bp == 0xff) {
+    /* bp is initialized to start - 1 in opj_mqc_init_enc() */
+    /* but this is safe, see opj_tcd_code_block_enc_allocate_data() */
+    assert(mqc->bp >= mqc->start - 1);
+    if (*mqc->bp == 0xff) {
         mqc->bp++;
         *mqc->bp = (OPJ_BYTE)(mqc->c >> 20);
         mqc->c &= 0xfffff;
@@ -296,12 +293,23 @@ OPJ_UINT32 opj_mqc_numbytes(opj_mqc_t *mqc)
 
 void opj_mqc_init_enc(opj_mqc_t *mqc, OPJ_BYTE *bp)
 {
-    /* TODO MSD: need to take a look to the v2 version */
+    /* To avoid the curctx pointer to be dangling, but not strictly */
+    /* required as the current context is always set before encoding */
     opj_mqc_setcurctx(mqc, 0);
+
+    /* As specified in Figure C.10 - Initialization of the encoder */
+    /* (C.2.8 Initialization of the encoder (INITENC)) */
     mqc->a = 0x8000;
     mqc->c = 0;
+    /* Yes, we point before the start of the buffer, but this is safe */
+    /* given opj_tcd_code_block_enc_allocate_data() */
     mqc->bp = bp - 1;
     mqc->ct = 12;
+    /* At this point we should test *(mqc->bp) against 0xFF, but this is not */
+    /* necessary, as this is only used at the beginning of the code block */
+    /* and our initial fake byte is set at 0 */
+    assert(*(mqc->bp) != 0xff);
+
     mqc->start = bp;
 }
 
@@ -316,60 +324,102 @@ void opj_mqc_encode(opj_mqc_t *mqc, OPJ_UINT32 d)
 
 void opj_mqc_flush(opj_mqc_t *mqc)
 {
+    /* C.2.9 Termination of coding (FLUSH) */
+    /* Figure C.11 – FLUSH procedure */
     opj_mqc_setbits(mqc);
     mqc->c <<= mqc->ct;
     opj_mqc_byteout(mqc);
     mqc->c <<= mqc->ct;
     opj_mqc_byteout(mqc);
 
+    /* It is forbidden that a coding pass ends with 0xff */
     if (*mqc->bp != 0xff) {
+        /* Advance pointer so that opj_mqc_numbytes() returns a valid value */
         mqc->bp++;
     }
 }
 
+#define BYPASS_CT_INIT  0xDEADBEEF
+
 void opj_mqc_bypass_init_enc(opj_mqc_t *mqc)
 {
+    /* This function is normally called after at least one opj_mqc_flush() */
+    /* which will have advance mqc->bp by at least 2 bytes beyond its */
+    /* initial position */
+    assert(mqc->bp >= mqc->start);
     mqc->c = 0;
-    mqc->ct = 8;
-    /*if (*mqc->bp == 0xff) {
-    mqc->ct = 7;
-     } */
+    /* in theory we should initialize to 8, but use this special value */
+    /* as a hint that opj_mqc_bypass_enc() has never been called, so */
+    /* as to avoid the 0xff 0x7f elimination trick in opj_mqc_bypass_flush_enc() */
+    /* to trigger when we don't have output any bit during this bypass sequence */
+    /* Any value > 8 will do */
+    mqc->ct = BYPASS_CT_INIT;
+    /* Given that we are called after opj_mqc_flush(), the previous byte */
+    /* cannot be 0xff. */
+    assert(mqc->bp[-1] != 0xff);
 }
 
 void opj_mqc_bypass_enc(opj_mqc_t *mqc, OPJ_UINT32 d)
 {
+    if (mqc->ct == BYPASS_CT_INIT) {
+        mqc->ct = 8;
+    }
     mqc->ct--;
     mqc->c = mqc->c + (d << mqc->ct);
     if (mqc->ct == 0) {
-        mqc->bp++;
         *mqc->bp = (OPJ_BYTE)mqc->c;
         mqc->ct = 8;
+        /* If the previous byte was 0xff, make sure that the next msb is 0 */
         if (*mqc->bp == 0xff) {
             mqc->ct = 7;
         }
+        mqc->bp++;
         mqc->c = 0;
     }
 }
 
-OPJ_UINT32 opj_mqc_bypass_flush_enc(opj_mqc_t *mqc)
+OPJ_UINT32 opj_mqc_bypass_get_extra_bytes(opj_mqc_t *mqc, OPJ_BOOL erterm)
 {
-    OPJ_BYTE bit_padding;
-
-    bit_padding = 0;
+    return (mqc->ct < 7 ||
+            (mqc->ct == 7 && (erterm || mqc->bp[-1] != 0xff))) ? 1 : 0;
+}
 
-    if (mqc->ct != 0) {
+void opj_mqc_bypass_flush_enc(opj_mqc_t *mqc, OPJ_BOOL erterm)
+{
+    /* Is there any bit remaining to be flushed ? */
+    /* If the last output byte is 0xff, we can discard it, unless */
+    /* erterm is required (I'm not completely sure why in erterm */
+    /* we must output 0xff 0x2a if the last byte was 0xff instead of */
+    /* discarding it, but Kakadu requires it when decoding */
+    /* in -fussy mode) */
+    if (mqc->ct < 7 || (mqc->ct == 7 && (erterm || mqc->bp[-1] != 0xff))) {
+        OPJ_BYTE bit_value = 0;
+        /* If so, fill the remaining lsbs with an alternating sequence of */
+        /* 0,1,... */
+        /* Note: it seems the standard only requires that for a ERTERM flush */
+        /* and doesn't specify what to do for a regular BYPASS flush */
         while (mqc->ct > 0) {
             mqc->ct--;
-            mqc->c += (OPJ_UINT32)(bit_padding << mqc->ct);
-            bit_padding = (bit_padding + 1) & 0x01;
+            mqc->c += (OPJ_UINT32)(bit_value << mqc->ct);
+            bit_value = (OPJ_BYTE)(1U - bit_value);
         }
-        mqc->bp++;
         *mqc->bp = (OPJ_BYTE)mqc->c;
-        mqc->ct = 8;
-        mqc->c = 0;
+        /* Advance pointer so that opj_mqc_numbytes() returns a valid value */
+        mqc->bp++;
+    } else if (mqc->ct == 7 && mqc->bp[-1] == 0xff) {
+        /* Discard last 0xff */
+        assert(!erterm);
+        mqc->bp --;
+    } else if (mqc->ct == 8 && !erterm &&
+               mqc->bp[-1] == 0x7f && mqc->bp[-2] == 0xff) {
+        /* Tiny optimization: discard terminating 0xff 0x7f since it is */
+        /* interpreted as 0xff 0x7f [0xff 0xff] by the decoder, and given */
+        /* the bit stuffing, in fact as 0xff 0xff [0xff ..] */
+        /* Happens once on opj_compress -i ../MAPA.tif -o MAPA.j2k  -M 1 */
+        mqc->bp -= 2;
     }
 
-    return 1;
+    assert(mqc->bp[-1] != 0xff);
 }
 
 void opj_mqc_reset_enc(opj_mqc_t *mqc)
@@ -380,6 +430,7 @@ void opj_mqc_reset_enc(opj_mqc_t *mqc)
     opj_mqc_setstate(mqc, T1_CTXNO_ZC, 0, 4);
 }
 
+#ifdef notdef
 OPJ_UINT32 opj_mqc_restart_enc(opj_mqc_t *mqc)
 {
     OPJ_UINT32 correction = 1;
@@ -396,15 +447,23 @@ OPJ_UINT32 opj_mqc_restart_enc(opj_mqc_t *mqc)
 
     return correction;
 }
+#endif
 
 void opj_mqc_restart_init_enc(opj_mqc_t *mqc)
 {
     /* <Re-init part> */
-    opj_mqc_setcurctx(mqc, 0);
+
+    /* As specified in Figure C.10 - Initialization of the encoder */
+    /* (C.2.8 Initialization of the encoder (INITENC)) */
     mqc->a = 0x8000;
     mqc->c = 0;
     mqc->ct = 12;
-    mqc->bp--;
+    /* This function is normally called after at least one opj_mqc_flush() */
+    /* which will have advance mqc->bp by at least 2 bytes beyond its */
+    /* initial position */
+    mqc->bp --;
+    assert(mqc->bp >= mqc->start - 1);
+    assert(*mqc->bp != 0xff);
     if (*mqc->bp == 0xff) {
         mqc->ct = 13;
     }