View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000202JVT JM H.264/AVC reference softwareencoder and decoderpublic2010-01-27 03:252010-05-27 14:05
Reporterllzzyynjupt Assigned ToKarsten Suehring  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product VersionJM 16.2 
Fixed in VersionJM-17.0 
Summary0000202: updated (possible) bug report about lossless intra coding
DescriptionNow I'm trying to learn H.264 lossless coding, and I begin my learning with lossless intra coding. But when I study the source codes of JM16.2, I find there exist many possible bugs about the lossless coding part of the source codes, which are listed in the following.

Encoder Side:

Problem 1: In the function “dct_16x16_ls()” of the file “block.c”, DPCM of size 4x4 is performed. But according to the H.264 standard (2009/03), only DPCM of size 16x16 may be used for Intra16x16. So in this function DPCM of size 4x4 should be replaced with DPCM of size16x16.

Problem 2: In the function “dct_chroma_ls()” of the file “block.c”, no DPCM is performed for the Chroma components. But according to the H.264 standard (2009/03), DPCM of size MbWidthCxMbHeightC is necessary in some cases. So DPCM of size MbWidthCxMbHeightC should be added to this function.
Also in this function there are codes like this:
Code Fragment Start:
        level = iabs(mb_ores[n2+j][n1+i]);
        mb_rres[n2+j][n1+i] = mb_ores[n2+j][n1+i];
……
        ACLevel[scan_pos ] = isignab(level, mb_ores[n2+j][n1+i]);
……
        level = isignab(level, mb_ores[n2+j][n1+i]);
Code Fragment End.
I think they should be replaced respectively with
Code Fragment Start:
level = iabs(mb_rres[n2+j][n1+i]);
……
ACLevel[scan_pos ] = isignab(level, mb_rres[n2+j][n1+i]);
……
level = isignab(level, mb_rres[n2+j][n1+i]);
Code Fragment End.
Besides, in this function, the “cr_cbp” isn’t updated when the AC coefficients are scanned. I think when nonzero AC coefficients are found, the following statement should be added:
Code Fragment Start:
cr_cbp=imax(2, cr_cbp);
Code Fragment End.

Problem 3: In the function "rdcost_for_4x4_intra_blocks()" of the file "rdopt.c", there are codes like this:
Code Fragment Start:
  //===== perform DCT, Q, IQ, IDCT, Reconstruction =====
  //select_dct(currMB);

  if (currMB->mb_type == I4MB)
      *nonzero = dct_4x4 (currMB, PLANE_Y, block_x, block_y, &dummy, 1);
  else
      *nonzero = currMB->trans_4x4 (currMB, PLANE_Y, block_x, block_y, &dummy, 1);
Code Fragment End.
It's obvious in this function, "currMB->mb_type == I4MB" will always be "TRUE" and the second branch will never be executed. So if we want to perform I4x4 lossless coding, the lossy coding function "dct_4x4()" will be executed and an error will be encountered.
As far as I know, "currMB->trans_4x4" is a function pointer. If lossy coding is performed, it will point to the lossy coding function "dct_4x4()". But if lossless coding is performed, it will point to the lossless coding function "dct_4x4_ls()". So I think the code fragment should be replaced with this:
Code Fragment Start:
  currMB->ipmode_DPCM = ipmode ;
  *nonzero = currMB->trans_4x4 (currMB, PLANE_Y, block_x, block_y, &dummy, 1);
Code Fragment End.
where “currMB->ipmode_DPCM = ipmode” is added because “currMB->ipmode_DPCM” isn’t set in advance in JM16.2.

Problem 4: In the function “dct_8x8_ls()” of the file “transform8x8.c”, there are codes like this:
Code Fragment Start:
if( (currMB->ipmode_DPCM < 2)&&(intra))
  {
    Residual_DPCM_8x8(currMB->ipmode_DPCM, mb_ores, mb_rres, block_y, block_x);
  }
Code Fragment End.
From the code fragment, we can see that when “(currMB->ipmode_DPCM < 2)&&(intra)” is “FALSE”, the variable “mb_rres”, which points to reconstructed residues, will point to uninitialized memory. And through experiments I found that when “(currMB->ipmode_DPCM < 2)&&(intra)” is “FALSE”, “mb_rres” pointed to stochastic values.
So I think the problem in the function “dct_8x8_ls()” of the file “transform8x8.c” can be solved by adding this before possible DPCM are performed:
Code Fragment Start:
for (j = block_y ; j < block_y + BLOCK_SIZE_8x8 ; j ++)
      for (i = block_x ; i < block_x + BLOCK_SIZE_8x8 ; i ++)
          mb_rres[j][i] = mb_ores[j][i] ;
Code Fragment End.

Decoder Side:

Problem 1: In the function “itrans4x4_ls()” of the file “block.c”, there’re codes like this:
Code Fragment Start:
inverse4x4(currSlice->cof[pl], mb_rres, joff, ioff);
Code Fragment End.
Obviously, this statement should be removed, as inverse DCT is not needed in lossless decoding. Also in this function, there’re codes like this:
Code Fragment Start:
mb_rec[j][i] = (imgpel) iClip1(max_imgpel_value, mb_pred[j][i] + rshift_rnd_sf(mb_rres[j][i], DQ_BITS));
Code Fragment End.
Obviously, the method of reconstruction is wrong, and the statement should be replaced by
Code Fragment Start:
mb_rec[j][i] = (imgpel) iClip1(max_imgpel_value, mb_pred[j][i] + mb_rres[j][i]);
Code Fragment End.

Problem 2: The function “iMBtrans4x4()” of the file “block.c” is called in the case of Intra16x16 decoding. In the function, “currMB->itrans_4x4(currMB, pl, ii, jj)” is executed to perform inverse DPCM of size 4x4. But according to the H.264 (2009/03), only inverse DPCM of size 16x16 may be used in some cases. So it’s necessary to replace inverse DPCM of size 4x4 with inverse DPCM of size 16x16 in this function.

Problem 3: In the function “read_CBP_and_coeffs_from_NAL_CABAC()” and the function “read_CBP_and_coeffs_from_NAL_CAVLC()” of the “macroblock.c” file, there are codes like this:
Code Fragment Start:
            //currSlice->cof[uv + 1][4][0] = currSlice->cofu[1];
            //currSlice->cof[uv + 1][0][4] = currSlice->cofu[2];
Code Fragment End.
Obviously, this is a typing bug. And in these two functions, the statements should be replaced with
Code Fragment Start:
            currSlice->cof[uv + 1][0][4] = currSlice->cofu[1];
            currSlice->cof[uv + 1][4][0] = currSlice->cofu[2];
Code Fragment End.

Problem 4: In the function “intra_cr_decoding()” of the file “mc_prediction.c”, no inverse DPCM is perform. But according to the H.264 (2009/03), inverse DPCM of size MbWidthCxMBHeightC should be performed in some cases. So inverse DPCM of size MbWidthCxMBHeightC should be added to this function.


TagsNo tags attached.

Relationships

child of 0000198 resolvedKarsten Suehring several possible bugs in the case of lossles coding 

Activities

2010-01-27 03:25

 

encoder.cfg (45,938 bytes)

Alexander Chuikov

2010-02-23 14:49

reporter   ~0000346

Hello llzzyynjupt,

I see this is the configuration file issue. I enable the lossless encode by configuration file changes, without any source code modification.
You should set correct profile, QP and some other options.

Karsten Suehring

2010-05-27 14:05

administrator   ~0000368

The fix has been included in JM 17.0

Issue History

Date Modified Username Field Change
2010-01-27 03:25 llzzyynjupt New Issue
2010-01-27 03:25 llzzyynjupt File Added: encoder.cfg
2010-02-23 14:49 Alexander Chuikov Note Added: 0000346
2010-05-27 14:05 Karsten Suehring Note Added: 0000368
2010-05-27 14:05 Karsten Suehring Status new => resolved
2010-05-27 14:05 Karsten Suehring Fixed in Version => JM-17.0
2010-05-27 14:05 Karsten Suehring Resolution open => fixed
2010-05-27 14:05 Karsten Suehring Assigned To => Karsten Suehring
2010-05-27 14:08 Karsten Suehring Relationship added child of 0000198