From 8d7f6600d6b6749edaa06a9fb7a44b4bf6f5840e Mon Sep 17 00:00:00 2001
From: Palanivel Guruvareddiar <palanivel.guruvareddiar@intel.com>
Date: Sun, 13 Dec 2020 14:04:10 -0700
Subject: [PATCH] Incorporating Karsten's review comments for ARSEI and also
 rebased with master.

---
 source/App/DecoderApp/DecApp.cpp     | 26 ++++++++++-----------
 source/Lib/CommonLib/TypeDef.h       |  5 +---
 source/Lib/EncoderLib/SEIEncoder.cpp | 35 +++++++++++-----------------
 3 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/source/App/DecoderApp/DecApp.cpp b/source/App/DecoderApp/DecApp.cpp
index 7b6b301e0..12271932f 100644
--- a/source/App/DecoderApp/DecApp.cpp
+++ b/source/App/DecoderApp/DecApp.cpp
@@ -1014,37 +1014,37 @@ void DecApp::xOutputAnnotatedRegions(PicList* pcListPic)
 
       if (!m_arObjects.empty())
       {
-        FILE *fp_persist = fopen(m_annotatedRegionsSEIFileName.c_str(), "ab");
-        if (fp_persist == NULL)
+        FILE *fpPersist = fopen(m_annotatedRegionsSEIFileName.c_str(), "ab");
+        if (fpPersist == NULL)
         {
           std::cout << "Not able to open file for writing persist SEI messages" << std::endl;
         }
         else
         {
-          fprintf(fp_persist, "\n");
-          fprintf(fp_persist, "Number of objects = %d\n", (int)m_arObjects.size());
+          fprintf(fpPersist, "\n");
+          fprintf(fpPersist, "Number of objects = %d\n", (int)m_arObjects.size());
           for (auto it = m_arObjects.begin(); it != m_arObjects.end(); ++it)
           {
-            fprintf(fp_persist, "Object Idx = %d\n",    it->first);
-            fprintf(fp_persist, "Object Top = %d\n",    it->second.boundingBoxTop);
-            fprintf(fp_persist, "Object Left = %d\n",   it->second.boundingBoxLeft);
-            fprintf(fp_persist, "Object Width = %d\n",  it->second.boundingBoxWidth);
-            fprintf(fp_persist, "Object Height = %d\n", it->second.boundingBoxHeight);
+            fprintf(fpPersist, "Object Idx = %d\n",    it->first);
+            fprintf(fpPersist, "Object Top = %d\n",    it->second.boundingBoxTop);
+            fprintf(fpPersist, "Object Left = %d\n",   it->second.boundingBoxLeft);
+            fprintf(fpPersist, "Object Width = %d\n",  it->second.boundingBoxWidth);
+            fprintf(fpPersist, "Object Height = %d\n", it->second.boundingBoxHeight);
             if (it->second.objectLabelValid)
             {
               auto labelIt=m_arLabels.find(it->second.objLabelIdx);
-              fprintf(fp_persist, "Object Label = %s\n", labelIt!=m_arLabels.end() ? (labelIt->second.c_str()) : "<UNKNOWN>");
+              fprintf(fpPersist, "Object Label = %s\n", labelIt!=m_arLabels.end() ? (labelIt->second.c_str()) : "<UNKNOWN>");
             }
             if (m_arHeader.m_partialObjectFlagPresentFlag)
             {
-              fprintf(fp_persist, "Object Partial = %d\n", it->second.partialObjectFlag?1:0);
+              fprintf(fpPersist, "Object Partial = %d\n", it->second.partialObjectFlag?1:0);
             }
             if (m_arHeader.m_objectConfidenceInfoPresentFlag)
             {
-              fprintf(fp_persist, "Object Conf = %d\n", it->second.objectConfidence);
+              fprintf(fpPersist, "Object Conf = %d\n", it->second.objectConfidence);
             }
           }
-          fclose(fp_persist);
+          fclose(fpPersist);
         }
       }
     }
diff --git a/source/Lib/CommonLib/TypeDef.h b/source/Lib/CommonLib/TypeDef.h
index cc905b533..10e6fc963 100644
--- a/source/Lib/CommonLib/TypeDef.h
+++ b/source/Lib/CommonLib/TypeDef.h
@@ -71,6 +71,7 @@
 #define JVET_R0046_IRAP_ASPECT2                           1 // Add a constraint on an ILRP being either an IRAP picture or having TemporalId less than or equal to Max (0, vps_max_tid_il_ref_pics_plus1[ refPicVpsLayerId ] - 1 )
 #define JVET_T0064                                        1 // JVET-T0064: control of filter strength for ALF
 #define JVET_R0264_IRAP_CONSTRAINT                        1 // when max_tid_il_ref_pic_plus1[i][j] is equal to 0, it would be prohibited to have mixtures of IRAP and non-IRAP NAL units in the picture
+#define JVET_T0053_ANNOTATED_REGIONS_SEI                  1 //Enable/disable the annotated regions SEI 
 
 //########### place macros to be be kept below this line ###############
 #define JVET_S0257_DUMP_360SEI_MESSAGE                    1 // Software support of 360 SEI messages
@@ -101,10 +102,6 @@ typedef std::pair<int, int>  TrCost;
 #define EXTENSION_360_VIDEO                               0   ///< extension for 360/spherical video coding support; this macro should be controlled by makefile, as it would be used to control whether the library is built and linked
 #endif
 
-#ifndef JVET_T0053_ANNOTATED_REGIONS_SEI
-#define JVET_T0053_ANNOTATED_REGIONS_SEI                  1 //Enable/disable the annotated regions SEI 
-#endif
-
 #ifndef EXTENSION_HDRTOOLS
 #define EXTENSION_HDRTOOLS                                0 //< extension for HDRTools/Metrics support; this macro should be controlled by makefile, as it would be used to control whether the library is built and linked
 #endif
diff --git a/source/Lib/EncoderLib/SEIEncoder.cpp b/source/Lib/EncoderLib/SEIEncoder.cpp
index 781b1135b..ddfed81b2 100644
--- a/source/Lib/EncoderLib/SEIEncoder.cpp
+++ b/source/Lib/EncoderLib/SEIEncoder.cpp
@@ -564,33 +564,24 @@ static void readTokenValueAndValidate(T            &returnedValue, /// value ret
 }
 
 #if JVET_T0053_ANNOTATED_REGIONS_SEI
-// Bool version does not have maximum and minimum values.
-static void readTokenValueAndValidate(bool         &returnedValue, /// value returned
-                                      bool         &failed,        /// used and updated
-                                      std::istream &is,            /// stream to read token from
-                                      const char  *pToken)        /// token string
-{
-  readTokenValue(returnedValue, failed, is, pToken);
-}
-
 void SEIEncoder::readAnnotatedRegionSEI(std::istream &fic, SEIAnnotatedRegions *seiAnnoRegion, bool &failed)
 {
-  readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_cancelFlag, failed, fic, "SEIArCancelFlag");
+  readTokenValue(seiAnnoRegion->m_hdr.m_cancelFlag, failed, fic, "SEIArCancelFlag");
   if (!seiAnnoRegion->m_hdr.m_cancelFlag)
   {
-    readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_notOptimizedForViewingFlag, failed, fic, "SEIArNotOptForViewingFlag");
-    readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_trueMotionFlag, failed, fic, "SEIArTrueMotionFlag");
-    readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_occludedObjectFlag, failed, fic, "SEIArOccludedObjsFlag");
-    readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_partialObjectFlagPresentFlag, failed, fic, "SEIArPartialObjsFlagPresentFlag");
-    readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_objectLabelPresentFlag, failed, fic, "SEIArObjLabelPresentFlag");
-    readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_objectConfidenceInfoPresentFlag, failed, fic, "SEIArObjConfInfoPresentFlag");
+    readTokenValue(seiAnnoRegion->m_hdr.m_notOptimizedForViewingFlag, failed, fic, "SEIArNotOptForViewingFlag");
+    readTokenValue(seiAnnoRegion->m_hdr.m_trueMotionFlag, failed, fic, "SEIArTrueMotionFlag");
+    readTokenValue(seiAnnoRegion->m_hdr.m_occludedObjectFlag, failed, fic, "SEIArOccludedObjsFlag");
+    readTokenValue(seiAnnoRegion->m_hdr.m_partialObjectFlagPresentFlag, failed, fic, "SEIArPartialObjsFlagPresentFlag");
+    readTokenValue(seiAnnoRegion->m_hdr.m_objectLabelPresentFlag, failed, fic, "SEIArObjLabelPresentFlag");
+    readTokenValue(seiAnnoRegion->m_hdr.m_objectConfidenceInfoPresentFlag, failed, fic, "SEIArObjConfInfoPresentFlag");
     if (seiAnnoRegion->m_hdr.m_objectConfidenceInfoPresentFlag)
     {
       readTokenValueAndValidate<uint32_t>(seiAnnoRegion->m_hdr.m_objectConfidenceLength, failed, fic, "SEIArObjDetConfLength", uint32_t(0), uint32_t(255));
     }
     if (seiAnnoRegion->m_hdr.m_objectLabelPresentFlag)
     {
-      readTokenValueAndValidate(seiAnnoRegion->m_hdr.m_objectLabelLanguagePresentFlag, failed, fic, "SEIArObjLabelLangPresentFlag");
+      readTokenValue(seiAnnoRegion->m_hdr.m_objectLabelLanguagePresentFlag, failed, fic, "SEIArObjLabelLangPresentFlag");
       if (seiAnnoRegion->m_hdr.m_objectLabelLanguagePresentFlag)
       {
         readTokenValue(seiAnnoRegion->m_hdr.m_annotatedRegionsObjectLabelLang, failed, fic, "SEIArLabelLanguage");
@@ -603,7 +594,7 @@ void SEIEncoder::readAnnotatedRegionSEI(std::istream &fic, SEIAnnotatedRegions *
         SEIAnnotatedRegions::AnnotatedRegionLabel &ar=it->second;
         readTokenValueAndValidate(it->first, failed, fic, "SEIArLabelIdc[c]", uint32_t(0), uint32_t(255));
         bool cancelFlag;
-        readTokenValueAndValidate(cancelFlag, failed, fic, "SEIArLabelCancelFlag[c]");
+        readTokenValue(cancelFlag, failed, fic, "SEIArLabelCancelFlag[c]");
         ar.labelValid=!cancelFlag;
         if (ar.labelValid)
         {
@@ -619,19 +610,19 @@ void SEIEncoder::readAnnotatedRegionSEI(std::istream &fic, SEIAnnotatedRegions *
     {
       SEIAnnotatedRegions::AnnotatedRegionObject &ar = it->second;
       readTokenValueAndValidate(it->first, failed, fic, "SEIArObjIdx[c]", uint32_t(0), uint32_t(255));
-      readTokenValueAndValidate(ar.objectCancelFlag, failed, fic, "SEIArObjCancelFlag[c]");
+      readTokenValue(ar.objectCancelFlag, failed, fic, "SEIArObjCancelFlag[c]");
       ar.objectLabelValid=false;
       ar.boundingBoxValid=false;
       if (!ar.objectCancelFlag)
       {
         if (seiAnnoRegion->m_hdr.m_objectLabelPresentFlag)
         {
-          readTokenValueAndValidate(ar.objectLabelValid, failed, fic, "SEIArObjLabelUpdateFlag[c]");
+          readTokenValue(ar.objectLabelValid, failed, fic, "SEIArObjLabelUpdateFlag[c]");
           if (ar.objectLabelValid)
           {
             readTokenValueAndValidate<uint32_t>(ar.objLabelIdx, failed, fic, "SEIArObjectLabelIdc[c]", uint32_t(0), uint32_t(255));
           }
-          readTokenValueAndValidate(ar.boundingBoxValid, failed, fic, "SEIArBoundBoxUpdateFlag[c]");
+          readTokenValue(ar.boundingBoxValid, failed, fic, "SEIArBoundBoxUpdateFlag[c]");
           if (ar.boundingBoxValid)
           {
             readTokenValueAndValidate<uint32_t>(ar.boundingBoxTop, failed, fic, "SEIArObjTop[c]", uint32_t(0), uint32_t(0x7fffffff));
@@ -640,7 +631,7 @@ void SEIEncoder::readAnnotatedRegionSEI(std::istream &fic, SEIAnnotatedRegions *
             readTokenValueAndValidate<uint32_t>(ar.boundingBoxHeight, failed, fic, "SEIArObjHeight[c]", uint32_t(0), uint32_t(0x7fffffff));
             if (seiAnnoRegion->m_hdr.m_partialObjectFlagPresentFlag)
             {
-              readTokenValueAndValidate(ar.partialObjectFlag, failed, fic, "SEIArObjPartUpdateFlag[c]");
+              readTokenValue(ar.partialObjectFlag, failed, fic, "SEIArObjPartUpdateFlag[c]");
             }
             if (seiAnnoRegion->m_hdr.m_objectConfidenceInfoPresentFlag)
             {
-- 
GitLab