Opened 8 years ago

Closed 8 years ago

#14 closed defect (fixed)

uninitialised memory read in JEM1.0

Reported by: davijane Owned by: XiangLi
Priority: minor Milestone: HM-16.6-JEM-2.0
Component: JEM Version: HM-16.6-JEM-1.0
Keywords: Cc: ksuehring, XiangLi, vceg-experts@…

Description (last modified by ksuehring)

I run JEM1.0 code with valgrind and found the problem of uninitialized memory . The error is as below. It is generated with LCU16x16 All Intra setting.

==26659== Use of uninitialised value of size 8
==26659==    at 0x484FC7: ContextModel::getEntropyBits(short) (ContextModel.h:118)
==26659==    by 0x4AA525: TEncSbac::estCBFBit(estBitsSbacStruct*) (TEncSbac.cpp:2383)
==26659==    by 0x4AA43F: TEncSbac::estBit(estBitsSbacStruct*, int, int, ChannelType) (TEncSbac.cpp:2355)
==26659==    by 0x4A0F36: TEncEntropy::estimateBit(estBitsSbacStruct*, int, int, ChannelType) (TEncEntropy.cpp:944)
==26659==    by 0x4B045D: TEncSearch::xIntraCodingTUBlock(TComYuv*, TComYuv*, TComYuv*, short**, bool, unsigned int&, ComponentID, TComTU&, int, unsigned int*) (TEncSearch.cpp:1527)
==26659==    by 0x4B1237: TEncSearch::xRecurIntraCodingLumaQT_RSAF(TComYuv*, TComYuv*, TComYuv*, short**, unsigned int&, bool, double&, TComTU&) (TEncSearch.cpp:1924)
==26659==    by 0x4B1922: TEncSearch::xRecurIntraCodingLumaQT_RSAF(TComYuv*, TComYuv*, TComYuv*, short**, unsigned int&, bool, double&, TComTU&) (TEncSearch.cpp:2067)
==26659==    by 0x4B69CE: TEncSearch::estIntraPredLumaQT(TComDataCU*, TComYuv*, TComYuv*, TComYuv*, TComYuv*, short**) (TEncSearch.cpp:3669)
==26659==    by 0x49BDD1: TEncCu::xCheckRDCostIntra(TComDataCU*&, TComDataCU*&, double&, PartSize, int&) (TEncCu.cpp:2293)
==26659==    by 0x496969: TEncCu::xCompressCU(TComDataCU*&, TComDataCU*&, unsigned int, PartSize) (TEncCu.cpp:1077)
==26659==    by 0x4946A9: TEncCu::compressCtu(TComDataCU*) (TEncCu.cpp:398)
==26659==    by 0x45BE96: TEncSlice::compressSlice(TComPic*, bool, bool) (TEncSlice.cpp:855)

One bug I found is:

In TEnslice.cpp Ln 666,

  m_pcEntropyCoder->setEntropyCoder   ( m_pppcRDSbacCoder[0][CI_CURR_BEST] );
  m_pcEntropyCoder->resetEntropy      ( pcSlice );

here only the CI_CURR_BEST is initialized.

Later, in TEncSearch.cpp Line 2011 and Line 2591, in function xRecurIntraCodingLumaQT_RSAF() and xRecurIntraCodingLumaQT
The sbac status of CI_QT_TRAFO_ROOT loaded is possibly uninitialized.

#if COM16_C806_EMT
        if( bSaveEmtResults && ( uiSingleCbfLuma || !bAllIntra || !m_pcEncCfg->getUseFastIntraEMT() ) )
          m_pcRDGoOnSbacCoder->load ( m_pppcRDSbacCoder[ uiFullDepth ][ CI_QT_TRAFO_ROOT ] );

The above problem caused windows program produce different output from linux. But fixing it did not fix all the memory problem. there are other memory bugs need to be fixed.


Attachments (2)

mem1.txt (35.1 KB) - added by davijane 8 years ago.
valgrind mem report file
encoder_intra_jvet10_LCU16_d1.cfg (7.7 KB) - added by davijane 8 years ago.

Download all attachments as: .zip

Change history (6)

Changed 8 years ago by davijane

valgrind mem report file

comment:1 Changed 8 years ago by XiangLi

  • Owner set to XiangLi
  • Status changed from new to assigned

Hi Jane,

Thanks for the report. I did a local test with several frames of a WQVGA sequence. I got the same coding results under Windows and Linux. Could you provide cfg and command line settings so that I can duplicate your problem? In addition, please let me know your compilers under Windows and Linux.

Thanks, Xiang

Changed 8 years ago by davijane

comment:2 Changed 8 years ago by davijane

HI, li Xiang, thanks for looking into it.

Basically, what I reported seem to be two separate problems.

  1. The attached valgrind memory check report is generated from unmodified JEM1.0 by using command
valgrind --leak-check=yes  ../bin/TAppEncoderStaticd S15BQSquare.cfg -c encoder_intra_jvet10_LCU16.cfg -q 33
  1. As to the problem of windows and linux mismatch, it happened to the code I modified which I forced intra coding not split. For JEM1.0, I don’t have an example to repeat the mismatch just using JEM1.0. I think the bug can be seen by examining the code.

Basically TEncSearch.cpp Line #1850 CABAC state is only stored if bCheckSplit is true.

  if( bCheckFull )
      //----- store original entropy coding status -----
        m_pcRDGoOnSbacCoder->store(m_pppcRDSbacCoder[ uiFullDepth ][ CI_QT_TRAFO_ROOT ]);    // line 1850

But it is loaded several times for bfilter loop and emtloop. So if bCheckSplit is false, the CABAC state loaded will be uninitialized.

If you use attached config file which try to limit LCU to 16x16 and with minimum split, step into it using visual studio, the first time the code stop at line # 1850, you will see the CABAC state loaded from CI_QT_TRAFO_ROOT have some weird uninitialized numbers.
I think this bug can be fixed by eliminating the if(bCheckSplit) before store(m_pppcRDSbacCoder[ uiFullDepth ][ CI_QT_TRAFO_ROOT ]



Last edited 8 years ago by ksuehring (previous) (diff)

comment:3 Changed 8 years ago by ksuehring

  • Description modified (diff)

comment:4 Changed 8 years ago by XiangLi

  • Milestone set to HM-16.6-JEM-2.0
  • Resolution set to fixed
  • Status changed from assigned to closed

Fixed in r134

Note: See TracTickets for help on using tickets.