From 60285d06d529d3d9bba7a7c6eb127f09962016db Mon Sep 17 00:00:00 2001
From: Karsten Suehring <karsten.suehring@hhi.fraunhofer.de>
Date: Tue, 11 Feb 2020 22:21:03 +0100
Subject: [PATCH] Refactoring/Bugfix: PPS parsing and subpictures

- don't use SPS in PPS parsing/writing
- create a function for decoder parameter set consistency checks
- fix bug with JVET-Q0413 subpicture parsing changes
- may contain traces of coding style fixes
---
 source/App/Parcat/parcat.cpp        |  2 +-
 source/Lib/CommonLib/Slice.cpp      | 17 ++++++++++++++--
 source/Lib/CommonLib/Slice.h        |  1 +
 source/Lib/DecoderLib/DecLib.cpp    | 30 ++++++++++++++++++++++++++---
 source/Lib/DecoderLib/DecLib.h      |  1 +
 source/Lib/DecoderLib/VLCReader.cpp | 29 +++-------------------------
 source/Lib/DecoderLib/VLCReader.h   |  2 +-
 source/Lib/EncoderLib/EncGOP.cpp    |  6 +++---
 source/Lib/EncoderLib/EncGOP.h      |  2 +-
 source/Lib/EncoderLib/VLCWriter.cpp |  6 +-----
 source/Lib/EncoderLib/VLCWriter.h   |  2 +-
 11 files changed, 55 insertions(+), 43 deletions(-)

diff --git a/source/App/Parcat/parcat.cpp b/source/App/Parcat/parcat.cpp
index f444a697e..08a1b1d39 100644
--- a/source/App/Parcat/parcat.cpp
+++ b/source/App/Parcat/parcat.cpp
@@ -279,7 +279,7 @@ std::vector<uint8_t> filter_segment(const std::vector<uint8_t> & v, int idx, int
     {
       PPS* pps = new PPS();
       HLSReader.setBitstream( &inp_nalu.getBitstream() );
-      HLSReader.parsePPS( pps, &parameterSetManager );
+      HLSReader.parsePPS( pps );
       parameterSetManager.storePPS( pps, inp_nalu.getBitstream().getFifo() );
     }
 #if !JVET_Q0819_PH_CHANGES
diff --git a/source/Lib/CommonLib/Slice.cpp b/source/Lib/CommonLib/Slice.cpp
index 94b0c5405..17cafc1e0 100644
--- a/source/Lib/CommonLib/Slice.cpp
+++ b/source/Lib/CommonLib/Slice.cpp
@@ -2649,6 +2649,17 @@ void PPS::initRectSliceMap()
 #if JVET_O1143_SUBPIC_BOUNDARY
 void PPS::initSubPic(const SPS &sps)
 {
+  if (getSubPicIdMappingInPpsFlag())
+  {
+    // When signalled, the number of subpictures has to match in PPS and SPS
+    CHECK (getNumSubPics() != sps.getNumSubPics(), "pps_num_subpics_minus1 shall be equal to sps_num_subpics_minus1");
+  }
+  else
+  {
+    // When not signalled  set the numer equal for convenient access
+    setNumSubPics(sps.getNumSubPics());
+  }
+
   CHECK(getNumSubPics() > MAX_NUM_SUB_PICS, "Number of sub-pictures in picture exceeds valid range");
   m_subPics.resize(getNumSubPics());
   // m_ctuSize,  m_picWidthInCtu, and m_picHeightInCtu might not be initialized yet.
@@ -2688,6 +2699,8 @@ void PPS::initSubPic(const SPS &sps)
 
     m_subPics[i].setSubPicBottom(bottom);
     
+    m_subPics[i].clearCTUAddrList();
+    
     if (m_numSlicesInPic == 1)
     {
       CHECK(getNumSubPics() != 1, "only one slice in picture, but number of subpic is not one");
@@ -3474,8 +3487,8 @@ bool ParameterSetManager::activatePPS(int ppsId, bool isIRAP)
           m_vpsMap.setActive(0);
         }
 
-          m_spsMap.clear();
-          m_spsMap.setActive(spsId);
+        m_spsMap.clear();
+        m_spsMap.setActive(spsId);
         m_activeSPSId = spsId;
         m_ppsMap.clear();
         m_ppsMap.setActive(ppsId);
diff --git a/source/Lib/CommonLib/Slice.h b/source/Lib/CommonLib/Slice.h
index 5add6de93..cc822aa1f 100644
--- a/source/Lib/CommonLib/Slice.h
+++ b/source/Lib/CommonLib/Slice.h
@@ -790,6 +790,7 @@ public:
   uint32_t         getSubPicHeightInLumaSample()     const { return  m_subPicHeightInLumaSample;      }
 
   std::vector<uint32_t> getCtuAddrList  ()           const { return  m_ctuAddrInSubPic;           }
+  void                  clearCTUAddrList()                 { m_ctuAddrInSubPic.clear(); }
   void                  addCTUsToSubPic(std::vector<uint32_t> ctuAddrInSlice)
   {
     for (auto ctu:ctuAddrInSlice)
diff --git a/source/Lib/DecoderLib/DecLib.cpp b/source/Lib/DecoderLib/DecLib.cpp
index 0c0dbf244..d9fddd4ff 100644
--- a/source/Lib/DecoderLib/DecLib.cpp
+++ b/source/Lib/DecoderLib/DecLib.cpp
@@ -1065,6 +1065,12 @@ void DecLib::xActivateParameterSets( const int layerId )
     {
       THROW("Parameter set activation failed!");
     }
+    
+#if JVET_O1143_SUBPIC_BOUNDARY
+    PPS* nonconstPPS = m_parameterSetManager.getPPS(m_picHeader.getPPSId());
+    nonconstPPS->initSubPic(*sps);
+#endif
+
     m_parameterSetManager.getApsMap()->clear();
     for (int i = 0; i < ALF_CTB_MAX_NUM_APS; i++)
     {
@@ -1274,7 +1280,11 @@ void DecLib::xActivateParameterSets( const int layerId )
        deleteSEIs(m_SEIs);
      }
   }
+  xCheckParameterSetConstraints(layerId);
+}
 
+void DecLib::xCheckParameterSetConstraints(const int layerId)
+{
   // Conformance checks
   Slice *slice = m_pcPic->slices[m_uiSliceSegmentIdx];
   const SPS *sps = slice->getSPS();
@@ -1393,6 +1403,17 @@ void DecLib::xActivateParameterSets( const int layerId )
     }
   }
 #endif
+
+#if JVET_Q0114_CONSTRAINT_FLAGS
+  if (sps->getProfileTierLevel()->getConstraintInfo()->getOneTilePerPicConstraintFlag())
+  {
+    CHECK(pps->getNumTiles() != 1, "When one_tile_per_pic_constraint_flag is equal to 1, each picture shall contain only one tile");
+  }
+  if (sps->getProfileTierLevel()->getConstraintInfo()->getOneSlicePerPicConstraintFlag())
+  {
+    CHECK( pps->getNumSlicesInPic() != 0, "When one_slice_per_pic_constraint_flag is equal to 1, each picture shall contain only one slice");
+  }
+#endif
 }
 
 
@@ -1433,7 +1454,8 @@ void DecLib::xDecodePicHeader( InputNALUnit& nalu )
 bool DecLib::xDecodeSlice(InputNALUnit &nalu, int &iSkipFrame, int iPOCLastDisplay )
 {
 #if !JVET_Q0775_PH_IN_SH
-  if(m_picHeader.isValid() == false) {
+  if(m_picHeader.isValid() == false)
+  {
     return false;
   }
 #endif
@@ -1449,7 +1471,7 @@ bool DecLib::xDecodeSlice(InputNALUnit &nalu, int &iSkipFrame, int iPOCLastDispl
   }
   else
   {
-      CHECK(nalu.m_nalUnitType != m_pcPic->slices[m_uiSliceSegmentIdx - 1]->getNalUnitType(), "The value of NAL unit type shall be the same for all coded slice NAL units of a picture");
+    CHECK(nalu.m_nalUnitType != m_pcPic->slices[m_uiSliceSegmentIdx - 1]->getNalUnitType(), "The value of NAL unit type shall be the same for all coded slice NAL units of a picture");
     m_apcSlicePilot->copySliceInfo( m_pcPic->slices[m_uiSliceSegmentIdx-1] );
   }
 
@@ -1469,7 +1491,9 @@ bool DecLib::xDecodeSlice(InputNALUnit &nalu, int &iSkipFrame, int iPOCLastDispl
   }
 
   if (nalu.m_nalUnitType == NAL_UNIT_CODED_SLICE_GDR)
+  {
     CHECK(nalu.m_temporalId != 0, "Current GDR picture has TemporalId not equal to 0");
+  }
 
   m_HLSReader.setBitstream( &nalu.getBitstream() );
 #if JVET_Q0795_CCALF
@@ -2008,7 +2032,7 @@ void DecLib::xDecodePPS( InputNALUnit& nalu )
 {
   PPS* pps = new PPS();
   m_HLSReader.setBitstream( &nalu.getBitstream() );
-  m_HLSReader.parsePPS( pps, &m_parameterSetManager );
+  m_HLSReader.parsePPS( pps );
   pps->setLayerId( nalu.m_nuhLayerId );
   pps->setTemporalId( nalu.m_temporalId );
   m_parameterSetManager.storePPS( pps, nalu.getBitstream().getFifo() );
diff --git a/source/Lib/DecoderLib/DecLib.h b/source/Lib/DecoderLib/DecLib.h
index 1ce8827d0..4f60dc184 100644
--- a/source/Lib/DecoderLib/DecLib.h
+++ b/source/Lib/DecoderLib/DecLib.h
@@ -220,6 +220,7 @@ protected:
   void  xCreateLostPicture( int iLostPOC, const int layerId );
   void  xCreateUnavailablePicture(int iUnavailablePoc, bool longTermFlag, const int layerId, const bool interLayerRefPicFlag);
   void  xActivateParameterSets( const int layerId );
+  void  xCheckParameterSetConstraints( const int layerId );
   void      xDecodePicHeader( InputNALUnit& nalu );
   bool      xDecodeSlice(InputNALUnit &nalu, int &iSkipFrame, int iPOCLastDisplay);
   void      xDecodeVPS( InputNALUnit& nalu );
diff --git a/source/Lib/DecoderLib/VLCReader.cpp b/source/Lib/DecoderLib/VLCReader.cpp
index 66061f91d..1a0e032c8 100644
--- a/source/Lib/DecoderLib/VLCReader.cpp
+++ b/source/Lib/DecoderLib/VLCReader.cpp
@@ -405,7 +405,7 @@ void HLSyntaxReader::parseRefPicList(SPS* sps, ReferencePictureList* rpl)
   rpl->setNumberOfInterLayerPictures( numIlrp );
 }
 
-void HLSyntaxReader::parsePPS( PPS* pcPPS, ParameterSetManager *parameterSetManager )
+void HLSyntaxReader::parsePPS( PPS* pcPPS )
 {
 #if ENABLE_TRACING
   xTracePPSHeader ();
@@ -480,20 +480,6 @@ void HLSyntaxReader::parsePPS( PPS* pcPPS, ParameterSetManager *parameterSetMana
     }
   }
 
-#if JVET_O1143_SUBPIC_BOUNDARY
-  // set the value of pps_num_subpics_minus1 equal to sps_num_subpics_minus1
-  pcPPS->setNumSubPics(parameterSetManager->getSPS(pcPPS->getSPSId())->getNumSubPics());
-#endif
-
-
-#if JVET_Q0114_CONSTRAINT_FLAGS
-  SPS* sps = parameterSetManager->getSPS(pcPPS->getSPSId());
-  if (sps->getProfileTierLevel()->getConstraintInfo()->getOneTilePerPicConstraintFlag())
-  {
-    CHECK(pcPPS->getNumTiles() != 1, "When one_tile_per_pic_constraint_flag is equal to 1, each picture shall contain only one tile");
-  }
-#endif
-
   READ_FLAG( uiCode, "no_pic_partition_flag" );                       pcPPS->setNoPicPartitionFlag( uiCode == 1 );
   if(!pcPPS->getNoPicPartitionFlag())
   {
@@ -532,14 +518,8 @@ void HLSyntaxReader::parsePPS( PPS* pcPPS, ParameterSetManager *parameterSetMana
       int32_t tileIdx = 0;
 
       READ_UVLC( uiCode, "num_slices_in_pic_minus1" );                pcPPS->setNumSlicesInPic( uiCode + 1 );
-#if JVET_Q0114_CONSTRAINT_FLAGS
-      SPS* sps = parameterSetManager->getSPS(pcPPS->getSPSId());
-      if (sps->getProfileTierLevel()->getConstraintInfo()->getOneSlicePerPicConstraintFlag())
-      {
-        CHECK(uiCode != 0, "When one_slice_per_pic_constraint_flag is equal to 1, each picture shall contain only one slice");
-      }
-#endif
       CHECK(pcPPS->getNumSlicesInPic() > MAX_SLICES,                  "Number of slices in picture exceeds valid range");
+
       READ_CODE(1, uiCode, "tile_idx_delta_present_flag");            pcPPS->setTileIdxDeltaPresentFlag( uiCode == 1 );
       pcPPS->initRectSlices();
       
@@ -609,9 +589,6 @@ void HLSyntaxReader::parsePPS( PPS* pcPPS, ParameterSetManager *parameterSetMana
     READ_CODE(1, uiCode, "loop_filter_across_tiles_enabled_flag");    pcPPS->setLoopFilterAcrossTilesEnabledFlag( uiCode == 1 );
     READ_CODE(1, uiCode, "loop_filter_across_slices_enabled_flag");   pcPPS->setLoopFilterAcrossSlicesEnabledFlag( uiCode == 1 );
   }
-#if JVET_O1143_SUBPIC_BOUNDARY  
-  pcPPS->initSubPic(*sps);
-#endif
 
   READ_FLAG(uiCode, "entropy_coding_sync_enabled_flag");       pcPPS->setEntropyCodingSyncEnabledFlag(uiCode == 1);
   READ_FLAG( uiCode,   "cabac_init_present_flag" );            pcPPS->setCabacInitPresentFlag( uiCode ? true : false );
@@ -1427,7 +1404,7 @@ void HLSyntaxReader::parseSPS(SPS* pcSPS)
       else
       {
 #if JVET_Q0413_SKIP_LAST_SUBPIC_SIG
-        pcSPS->setSubPicWidth(picIdx, (pcSPS->getMaxPicHeightInLumaSamples() + pcSPS->getCTUSize() - 1) /pcSPS->getCTUSize() - pcSPS->getSubPicCtuTopLeftY(picIdx));
+        pcSPS->setSubPicHeight(picIdx, (pcSPS->getMaxPicHeightInLumaSamples() + pcSPS->getCTUSize() - 1) /pcSPS->getCTUSize() - pcSPS->getSubPicCtuTopLeftY(picIdx));
 #else
         pcSPS->setSubPicHeight(picIdx, 1);
 #endif
diff --git a/source/Lib/DecoderLib/VLCReader.h b/source/Lib/DecoderLib/VLCReader.h
index cf68f5d2d..a439a95be 100644
--- a/source/Lib/DecoderLib/VLCReader.h
+++ b/source/Lib/DecoderLib/VLCReader.h
@@ -158,7 +158,7 @@ public:
   void  parseVPS            ( VPS* pcVPS );
   void  parseDPS            ( DPS* dps );
   void  parseSPS            ( SPS* pcSPS );
-  void  parsePPS            ( PPS* pcPPS, ParameterSetManager *parameterSetManager );
+  void  parsePPS            ( PPS* pcPPS );
   void  parseAPS            ( APS* pcAPS );
   void  parseAlfAps         ( APS* pcAPS );
   void  parseLmcsAps        ( APS* pcAPS );
diff --git a/source/Lib/EncoderLib/EncGOP.cpp b/source/Lib/EncoderLib/EncGOP.cpp
index 5770d6d4e..1ba9a538c 100644
--- a/source/Lib/EncoderLib/EncGOP.cpp
+++ b/source/Lib/EncoderLib/EncGOP.cpp
@@ -347,13 +347,13 @@ int EncGOP::xWriteSPS( AccessUnit &accessUnit, const SPS *sps, const int layerId
 
 }
 
-int EncGOP::xWritePPS( AccessUnit &accessUnit, const PPS *pps, const SPS *sps, const int layerId )
+int EncGOP::xWritePPS( AccessUnit &accessUnit, const PPS *pps, const int layerId )
 {
   OutputNALUnit nalu(NAL_UNIT_PPS);
   m_HLSWriter->setBitstream( &nalu.m_Bitstream );
   nalu.m_nuhLayerId = layerId;
   CHECK( nalu.m_temporalId < accessUnit.temporalId, "TemporalId shall be greater than or equal to the TemporalId of the layer access unit containing the NAL unit" );
-  m_HLSWriter->codePPS( pps, sps );
+  m_HLSWriter->codePPS( pps );
   accessUnit.push_back(new NALUnitEBSP(nalu));
   return (int)(accessUnit.back()->m_nalUnitData.str().size()) * 8;
 }
@@ -407,7 +407,7 @@ int EncGOP::xWriteParameterSets(AccessUnit &accessUnit, Slice *slice, const bool
 
   if( m_pcEncLib->PPSNeedsWriting( slice->getPPS()->getPPSId() ) ) // Note this assumes that all changes to the PPS are made at the EncLib level prior to picture creation (EncLib::xGetNewPicBuffer).
   {
-    actualTotalBits += xWritePPS( accessUnit, slice->getPPS(), slice->getSPS(), m_pcEncLib->getLayerId() );
+    actualTotalBits += xWritePPS( accessUnit, slice->getPPS(), m_pcEncLib->getLayerId() );
   }
 
   return actualTotalBits;
diff --git a/source/Lib/EncoderLib/EncGOP.h b/source/Lib/EncoderLib/EncGOP.h
index 0561fadad..f84e7f566 100644
--- a/source/Lib/EncoderLib/EncGOP.h
+++ b/source/Lib/EncoderLib/EncGOP.h
@@ -321,7 +321,7 @@ protected:
   int xWriteVPS (AccessUnit &accessUnit, const VPS *vps);
   int xWriteDPS (AccessUnit &accessUnit, const DPS *dps);
   int xWriteSPS( AccessUnit &accessUnit, const SPS *sps, const int layerId = 0 );
-  int xWritePPS( AccessUnit &accessUnit, const PPS *pps, const SPS *sps, const int layerId = 0 );
+  int xWritePPS( AccessUnit &accessUnit, const PPS *pps, const int layerId = 0 );
   int xWriteAPS( AccessUnit &accessUnit, APS *aps, const int layerId, const bool isPrefixNUT );
 #if ENABLING_MULTI_SPS
   int xWriteParameterSets(AccessUnit &accessUnit, Slice *slice, const bool bSeqFirst, const int layerIdx);
diff --git a/source/Lib/EncoderLib/VLCWriter.cpp b/source/Lib/EncoderLib/VLCWriter.cpp
index 4857cc1c8..aa67ccdb5 100644
--- a/source/Lib/EncoderLib/VLCWriter.cpp
+++ b/source/Lib/EncoderLib/VLCWriter.cpp
@@ -242,7 +242,7 @@ void HLSWriter::xCodeRefPicList( const ReferencePictureList* rpl, bool isLongTer
   }
 }
 
-void HLSWriter::codePPS( const PPS* pcPPS, const SPS* pcSPS )
+void HLSWriter::codePPS( const PPS* pcPPS )
 {
 #if ENABLE_TRACING
   xTracePPSHeader ();
@@ -395,10 +395,6 @@ void HLSWriter::codePPS( const PPS* pcPPS, const SPS* pcSPS )
 #endif
   WRITE_SVLC( pcPPS->getQpOffset(COMPONENT_Cb), "pps_cb_qp_offset" );
   WRITE_SVLC( pcPPS->getQpOffset(COMPONENT_Cr), "pps_cr_qp_offset" );
-  if (pcSPS->getJointCbCrEnabledFlag() == false || pcSPS->getChromaFormatIdc() == CHROMA_400)
-  {
-    CHECK(pcPPS->getJointCbCrQpOffsetPresentFlag(), "pps_jcbcr_qp_offset_present_flag should be false");
-  }
   WRITE_FLAG(pcPPS->getJointCbCrQpOffsetPresentFlag() ? 1 : 0, "pps_joint_cbcr_qp_offset_present_flag");
   if (pcPPS->getJointCbCrQpOffsetPresentFlag())
   {
diff --git a/source/Lib/EncoderLib/VLCWriter.h b/source/Lib/EncoderLib/VLCWriter.h
index 01e8ed40e..7c3ffbce6 100644
--- a/source/Lib/EncoderLib/VLCWriter.h
+++ b/source/Lib/EncoderLib/VLCWriter.h
@@ -125,7 +125,7 @@ public:
   uint32_t  getNumberOfWrittenBits  ()                      { return m_pcBitIf->getNumberOfWrittenBits();  }
   void  codeVUI                 ( const VUI *pcVUI, const SPS* pcSPS );
   void  codeSPS                 ( const SPS* pcSPS );
-  void  codePPS                 ( const PPS* pcPPS, const SPS* pcSPS );
+  void  codePPS                 ( const PPS* pcPPS );
   void  codeAPS                 ( APS* pcAPS );
   void  codeAlfAps              ( APS* pcAPS );
   void  codeLmcsAps             ( APS* pcAPS );
-- 
GitLab