Skip to content

Commit

Permalink
fixed valgrind error where MQ coder was accessing uninitialized memor…
Browse files Browse the repository at this point in the history
…y in

RESTART mode. Unfortunately, there is still a problem with RESTART:
lossless encoding with RESTART produces a lossy image.
  • Loading branch information
Aaron Boxer committed Mar 18, 2016
1 parent b52e9db commit 3d9ee7a
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/lib/openjp2/mqc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ static opj_mqc_state_t mqc_states[47 * 2] = {

static void opj_mqc_byteout(opj_mqc_t *mqc)
{
if (mqc->bp == mqc->start - 1) {
if (mqc->bp < mqc->start) {
mqc->bp++;
*mqc->bp = (uint8_t)(mqc->c >> 19);
mqc->c &= 0x7ffff;
Expand Down
38 changes: 16 additions & 22 deletions src/lib/openjp2/t1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,9 @@ double opj_t1_encode_cblk(opj_t1_t *t1,
opj_mqc_setstate(mqc, T1_CTXNO_ZC, 0, 4);
opj_mqc_init_enc(mqc, cblk->data);

bool TERMALL = (cblksty & J2K_CCP_CBLKSTY_TERMALL);
bool LAZY = (cblksty & J2K_CCP_CBLKSTY_LAZY);

for (passno = 0; bpno >= 0; ++passno) {
opj_tcd_pass_t *pass = &cblk->passes[passno];
uint32_t correction = 3;
Expand All @@ -1592,53 +1595,44 @@ double opj_t1_encode_cblk(opj_t1_t *t1,
break;
}


/* fixed_quality */
tempwmsedec = opj_t1_getwmsedec(nmsedec, compno, level, orient, bpno, qmfbid, stepsize, numcomps,mct_norms, mct_numcomps) ;
cumwmsedec += tempwmsedec;

/* Code switch "RESTART" (i.e. TERMALL) */
if ((cblksty & J2K_CCP_CBLKSTY_TERMALL) && !((passtype == 2) && (bpno - 1 < 0))) {
if ( TERMALL ||
(LAZY && ((bpno < ((int32_t)(cblk->numbps) - 4) && (passtype > 0))
|| ((bpno == ((int32_t)cblk->numbps - 4)) && (passtype == 2))))
) {
if (type == T1_TYPE_RAW) {
opj_mqc_flush(mqc);
correction = 1;
/* correction = mqc_bypass_flush_enc(); */
} else { /* correction = mqc_restart_enc(); */
} else {
opj_mqc_flush(mqc);
correction = 1;
}
pass->term = 1;
} else {
if (((bpno < ((int32_t) (cblk->numbps) - 4) && (passtype > 0))
|| ((bpno == ((int32_t)cblk->numbps - 4)) && (passtype == 2))) && (cblksty & J2K_CCP_CBLKSTY_LAZY)) {
if (type == T1_TYPE_RAW) {
opj_mqc_flush(mqc);
correction = 1;
/* correction = mqc_bypass_flush_enc(); */
} else { /* correction = mqc_restart_enc(); */
opj_mqc_flush(mqc);
correction = 1;
}
pass->term = 1;
} else {
pass->term = 0;
}
pass->term = 0;
}

if (++passtype == 3) {
passtype = 0;
bpno--;
}

if (pass->term && bpno > 0) {
type = ((bpno < ((int32_t) (cblk->numbps) - 4)) && (passtype < 2) && (cblksty & J2K_CCP_CBLKSTY_LAZY)) ? T1_TYPE_RAW : T1_TYPE_MQ;
if (pass->term) {
type = ((bpno < ((int32_t) (cblk->numbps) - 4)) && (passtype < 2) && LAZY) ? T1_TYPE_RAW : T1_TYPE_MQ;
if (type == T1_TYPE_RAW)
opj_mqc_bypass_init_enc(mqc);
else
opj_mqc_restart_init_enc(mqc);
}

pass->distortiondec = cumwmsedec;
pass->rate = opj_mqc_numbytes(mqc) + correction; /* FIXME */
pass->rate = opj_mqc_numbytes(mqc);
if (pass->rate > 0)
pass->rate += correction;

/* Code-switch "RESET" */
if (cblksty & J2K_CCP_CBLKSTY_RESET)
Expand All @@ -1648,7 +1642,7 @@ double opj_t1_encode_cblk(opj_t1_t *t1,
/* Code switch "ERTERM" (i.e. PTERM) */
if (cblksty & J2K_CCP_CBLKSTY_PTERM)
opj_mqc_erterm_enc(mqc);
else /* Default coding */ if (!(cblksty & J2K_CCP_CBLKSTY_LAZY))
else if (!LAZY && !TERMALL)
opj_mqc_flush(mqc);

cblk->totalpasses = passno;
Expand Down

1 comment on commit 3d9ee7a

@boxerab
Copy link
Collaborator

@boxerab boxerab commented on 3d9ee7a May 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    if (pass->term && bpno > 0)

is the important line. For bpno == 0, there is no termination. That is why you are seeing the encoder go into an endless loop. Simply remove bpno > 0 and it should fix the valgrind error. Of course, lossless encoding may still be lossy, but that is a bigger issue.

Please sign in to comment.