View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0000228JVT JM H.264/AVC reference softwaredecoderpublic2010-08-03 11:522012-08-04 13:22
Reporterllzzyynjupt Assigned ToKarsten Suehring  
PrioritynormalSeveritymajorReproducibilityalways
Status resolvedResolutionfixed 
Product VersionJM 17.2 
Fixed in VersionJM 18.4 
Summary0000228: a bug about the lossless decoding of the JM 17.2 software
Description
If video frames are lossless encoded using the attached configuration file “encoder.cfg”, I find that the chroma components of inter coded frames (i.e. P and B frames) cannot be correctly decoded using the decoder of JM 17.2. The possible cause of this bug and the way of removing it is described as follows.

Problem 1.

Decoder Side:
In the function “read_CBP_and_coeffs_from_NAL_CABAC_420()” of the file “macroblock.c”, there are codes like this:

Code Fragment Start:
  //========================== CHROMA DC ============================
  //-----------------------------------------------------------------
  // chroma DC coeff
  if(cbp>15)
  {
    int uv, ll, k, coef_ctr;
    for (ll=0;ll<3;ll+=2)
    {
      uv = ll>>1;

      ……

      for(k = 0; (k < (p_Vid->num_cdc_coeff + 1))&&(level!=0);++k)
      {

        ……

        if (level != 0)
        {
          ……
          currSlice->cofu[coef_ctr]=level;
        }
      }


      if (smb || (currMB->is_lossless == TRUE)) // check to see if MB type is SPred or SIntra4x4
      {
        currSlice->cof[uv + 1][0][0] = currSlice->cofu[0];
        currSlice->cof[uv + 1][0][4] = currSlice->cofu[1];
        currSlice->cof[uv + 1][4][0] = currSlice->cofu[2];
        currSlice->cof[uv + 1][4][4] = currSlice->cofu[3];
        ……
      }
      else
      {
        ……
      }
    }
  }
Code Fragment End.

Still in this function, there are codes like this:

Code Fragment Start:
  //========================== CHROMA AC ============================
  //-----------------------------------------------------------------
  // chroma AC coeff, all zero fram start_scan
  if (cbp >31)
  {
    ……

    if(currMB->is_lossless == FALSE)
    {
      ……
    }
    else
    {
      int b4, b8, k;
      int uv;
      for (b8=0; b8 < p_Vid->num_blk8x8_uv; ++b8)
      {
        currMB->is_v_block = uv = (b8 > ((p_Vid->num_uv_blocks) - 1 ));

        for (b4=0; b4 < 4; ++b4)
        {
          ……

          for(k=0;(k<16)&&(level!=0);++k)
          {
            ……

            if (level != 0)
            {
              ……

              currSlice->cof[uv + 1][(j<<2) + j0][(i<<2) + i0] = level;
              ……
            }
          }
        }
      }
    } //for (b4=0; b4 < 4; b4++)
  }
Code Fragment End.

We can see that both the DC and AC coefficients of the chroma components are saved in “currSlice->cof”. In fact, from other functions, such as “read_CBP_and_coeffs_from_NAL_CABAC_422()”, we can also see that both the DC and AC coefficients of the chroma components are saved in “currSlice->cof”.

In the function “iTransform()” of the file “block.c”, there are codes like this:

Code Fragment Start:
  if ((dec_picture->chroma_format_idc != YUV400) && (dec_picture->chroma_format_idc != YUV444))
  {
    imgpel **curUV;
    int b8;
    int ioff, joff;
    imgpel **mb_rec;

    for(uv = PLANE_U; uv <= PLANE_V; ++uv)
    {
      // =============== 4x4 itrans ================
      // -------------------------------------------
      curUV = &dec_picture->imgUV[uv - 1][currMB->pix_c_y];
      mb_rec = currSlice->mb_rec[uv];

      if (!smb && (currMB->cbp>>4))
      {
        if (currMB->is_lossless == FALSE)
        {
          ……
        }
        else
        {
          const unsigned char *x_pos, *y_pos;
          for (b8 = 0; b8 < (p_Vid->num_uv_blocks); ++b8)
          {
            x_pos = subblk_offset_x[1][b8];
            y_pos = subblk_offset_y[1][b8];

            itrans4x4_ls(currMB, uv, *x_pos++, *y_pos++);
            itrans4x4_ls(currMB, uv, *x_pos++, *y_pos++);
            itrans4x4_ls(currMB, uv, *x_pos++, *y_pos++);
            itrans4x4_ls(currMB, uv, *x_pos , *y_pos );
          }
        }
        copy_image_data(curUV, mb_rec, currMB->pix_c_x, 0, p_Vid->mb_size[1][0], p_Vid->mb_size[1][1]);
      }
      else if (smb)
      {
        currMB->itrans_4x4 = (currMB->is_lossless == FALSE) ? itrans4x4 : itrans4x4_ls;
        itrans_sp_cr(currMB, uv - 1);

        for (joff = 0; joff < p_Vid->mb_cr_size_y; joff += BLOCK_SIZE)
        {
          for(ioff = 0; ioff < p_Vid->mb_cr_size_x ;ioff += BLOCK_SIZE)
          {
            currMB->itrans_4x4(currMB, uv, ioff, joff);
          }
        }

        copy_image_data(curUV, mb_rec, currMB->pix_c_x, 0, p_Vid->mb_size[1][0], p_Vid->mb_size[1][1]);
      }
      else
      {
        copy_image_data(curUV, currSlice->mb_pred[uv], currMB->pix_c_x, 0, p_Vid->mb_size[1][0], p_Vid->mb_size[1][1]);
      }
    }
  }
Code Fragment End.

It seems the function “iTransform()” is used to reconstruct inter coded frames. Here, the called function “itrans4x4_ls()” reconstructs the chroma components of inter coded frames using the data in “currSlice->mb_rres” but not the data in “currSlice->cof”. But in the case of lossless decoding, I find that before the function “itrans4x4_ls()” is called, the chroma data in “currSlice->cof” are not copied to “currSlice->mb_rres” in advance. So I think before the function “itrans4x4_ls()” is called, the following codes should be added:

Code Fragment Start:
{
                int i, j ;
                for (i = 0 ; i < p_Vid->mb_cr_size_y ; i ++)
                    for (j = 0 ; j < p_Vid->mb_cr_size_x ; j ++)
                        currSlice->mb_rres[uv][i][j] = currSlice->cof[uv][i][j] ;
}
Code Fragment End.

And after these codes are added, I find that all video frames can be lossless decoded correctly.


Suggestion.

Decoder Side:

Here I only take the function “read_CBP_and_coeffs_from_NAL_CABAC_420()” of the file “macroblock.c” for example.

In the function “read_CBP_and_coeffs_from_NAL_CABAC_420()” of the file “macroblock.c”, there are codes like this:

Code Fragment Start:
      for(k = 0; (k < 17) && (level != 0); ++k)
      {
#if TRACE
        snprintf(currSE.tracestring, TRACESTRING_SIZE, "DC luma 16x16 ");
#endif
        dP->readSyntaxElement(currMB, &currSE, dP);
        level = currSE.value1;

        if (level != 0) /* leave if level == 0 */
        {
          pos_scan_4x4 += (2 * currSE.value2);

          i0 = ((*pos_scan_4x4++) << 2);
          j0 = ((*pos_scan_4x4++) << 2);

          currSlice->cof[0][j0][i0] = level;// add new intra DC coeff
          //p_Vid->fcf[0][j0][i0] = level;// add new intra DC coeff
        }
      }
Code Fragment End.

So the DC coefficients of the luma component in the I16x16 mode are saved in “currSlice->cof”.

Still in this function, there are codes like this:

Code Fragment Start:
  // luma coefficients
  //======= Other Modes & CABAC ========
  //------------------------------------
  if (cbp)
  {
    if(currMB->luma_transform_size_8x8_flag)
    {
      //======= 8x8 transform size & CABAC ========
      readCompCoeff8x8MB_CABAC (currMB, &currSE, PLANE_Y);
    }
    else
    {
      InvLevelScale4x4 = intra? currSlice->InvLevelScale4x4_Intra[currSlice->colour_plane_id][qp_rem] : currSlice->InvLevelScale4x4_Inter[currSlice->colour_plane_id][qp_rem];
      readCompCoeff4x4MB_CABAC (currMB, &currSE, PLANE_Y, InvLevelScale4x4, qp_per, cbp);
    }
  }
Code Fragment End.

From the called function “readCompCoeff8x8MB_CABAC()” (and the further called functions “readCompCoeff8x8_CABAC()” and “readCompCoeff8x8_CABAC_lossless()” in it), I find the coefficients of the luma component are saved in “currSlice->mb_rres” if 8x8 transform is used.

From the called function “readCompCoeff4x4MB_CABAC()” (and the further called function “readCompCoeff4x4SMB_CABAC()” in it), I find the coefficients of the luma component are saved in “currSlice->cof” if 4x4 transform is used.

From the description of “Problem 1”, we have already known that the coefficients of the chroma components are saved in “currSlice->cof”.

So the fact that sometimes coefficients are saved in “currSlice->cof” and sometimes the coefficients are saved in “currSlice->mb_rres” makes the source codes quite inconsistent.

Besides, in the file “block.c” the functions “Inv_Residual_trans_16x16()”, “Inv_Residual_trans_4x4()”, “Inv_Residual_trans_8x8()” and “Inv_Residual_trans_Chroma()” are coded quite inconsistent, either. For example, in the function “Inv_Residual_trans_16x16()”, codes are organized like this:

Code Fragment Start:
  if(currMB->ipmode_DPCM == VERT_PRED_16)
  {
    ……
  }
  else if(currMB->ipmode_DPCM == HOR_PRED_16)
  {
    ……
  }
  else
  {
    for (j = 0; j < MB_BLOCK_SIZE; ++j)
      for (i = 0; i < MB_BLOCK_SIZE; ++i)
        mb_rres[j][i] = cof[j][i];
  }

  for (j = 0; j < MB_BLOCK_SIZE; ++j)
  {
    for (i = 0; i < MB_BLOCK_SIZE; ++i)
    {
      mb_rec[j][i] = (imgpel) (mb_rres[j][i] + mb_pred[j][i]);
    }
  }
Code Fragment End.

In the function “Inv_Residual_trans_4x4()”, codes are organized like this:

Code Fragment Start:
  if(currMB->ipmode_DPCM == VERT_PRED)
  {
    ……
  }
  else if(currMB->ipmode_DPCM == HOR_PRED)
  {
    ……
  }
  else
  {
    for (j = joff; j < joff + BLOCK_SIZE; ++j)
      for (i = ioff; i < ioff + BLOCK_SIZE; ++i)
        mb_rres[j][i] = cof[j][i];
  }

  for (j = joff; j < joff + BLOCK_SIZE; ++j)
  {
    for (i = ioff; i < ioff + BLOCK_SIZE; ++i)
    {
      mb_rec[j][i] = (imgpel) (mb_rres[j][i] + mb_pred[j][i]);
    }
  }
Code Fragment End.

In contrast, in the function “Inv_Residual_trans_8x8()”, codes are organized like this:

Code Fragment Start:
  if(currMB->ipmode_DPCM == VERT_PRED)
  {
    ……
  }
  else if(currMB->ipmode_DPCM == HOR_PRED)//HOR_PRED
  {
    ……
  }

  for (j = joff; j < joff + BLOCK_SIZE*2; ++j)
  {
    for (i = ioff; i < ioff + BLOCK_SIZE*2; ++i)
    {
      mb_rec [j][i] = (imgpel) (mb_rres[j][i] + mb_pred[j][i]);
    }
  }
Code Fragment End.

And in the function “Inv_Residual_trans_Chroma()”, codes are organized like this:

Code Fragment Start:
  if(currMB->c_ipred_mode == VERT_PRED_8)
  {
    ……
  }
  else //HOR_PRED_8
  {
    ……
  }
Code Fragment End.

So in the file “block.c” the functions “Inv_Residual_trans_16x16()”, “Inv_Residual_trans_4x4()”, “Inv_Residual_trans_8x8()” and “Inv_Residual_trans_Chroma()” are coded in an inconsistent manner. As a result, these functions are called in different ways. It seems this coding style makes the codes hard to understand and vulnerable to bugs.

So I suggest all the inconsistent codes be written in a more consistent manner in the future.


TagsNo tags attached.

Activities

2010-08-03 11:52

 

encoder.cfg (48,467 bytes)

Karsten Suehring

2012-08-04 13:22

administrator   ~0000558

fixed per patch from Bin Li / Microsoft

Issue History

Date Modified Username Field Change
2010-08-03 11:52 llzzyynjupt New Issue
2010-08-03 11:52 llzzyynjupt File Added: encoder.cfg
2012-08-04 13:22 Karsten Suehring Note Added: 0000558
2012-08-04 13:22 Karsten Suehring Status new => resolved
2012-08-04 13:22 Karsten Suehring Fixed in Version => JM 18.4
2012-08-04 13:22 Karsten Suehring Resolution open => fixed
2012-08-04 13:22 Karsten Suehring Assigned To => Karsten Suehring