Re: [x265] [PATCH] Fix memory leak in hdr10plus

2017-07-24 Thread Pradeep Ramachandran
On Mon, Jul 24, 2017 at 11:08 AM,  wrote:

> # HG changeset patch
> # User Bhavna Hariharan 
> # Date 1500874610 -19800
> #  Mon Jul 24 11:06:50 2017 +0530
> # Branch stable
> # Node ID c28a95a9ebbf5bf6bb5c9a357fc11e3c3bdea35c
> # Parent  bce945545c241ce4bb87d56d283ac8226d862ed5
> Fix memory leak in hdr10plus
>

> Rename numCimInfo and cim to m_numCimInfo and m_cim respectively.
>
> Fixes memory leaks cause due to incorrect handling of m_cim and m_userSEI
> memory allocation.
>
> Handle crash when empty json file is parsed.
>

Pushed on appropriate parent in stable branch and merged with latest stable
and default tips.


>
> diff -r bce945545c24 -r c28a95a9ebbf source/dynamicHDR10/JsonHelper.cpp
> --- a/source/dynamicHDR10/JsonHelper.cppWed Jun 07 16:29:15 2017
> +0530
> +++ b/source/dynamicHDR10/JsonHelper.cppMon Jul 24 11:06:50 2017
> +0530
> @@ -188,9 +188,15 @@
>
>  tfile.close();
>
> -size_t beginning = json_str2.find_first_of("[");
> -int fixchar = json_str2[json_str2.size() - 2] == ']' ? 1 : 0;
> -return Json::parse(json_str2.substr(beginning,json_str2.size() -
> fixchar),err).array_items();
> +vector data;
> +if (json_str2.size() != 0)
> +{
> +size_t beginning = json_str2.find_first_of("[");
> +int fixchar = json_str2[json_str2.size() - 2] == ']' ? 1 : 0;
> +return Json::parse(json_str2.substr(beginning, json_str2.size()
> - fixchar), err).array_items();
> +}
> +else
> +return data;
>  }
>
>  bool JsonHelper::validatePathExtension(string )
> diff -r bce945545c24 -r c28a95a9ebbf source/encoder/encoder.cpp
> --- a/source/encoder/encoder.cppWed Jun 07 16:29:15 2017 +0530
> +++ b/source/encoder/encoder.cppMon Jul 24 11:06:50 2017 +0530
> @@ -88,8 +88,8 @@
>
>  #if ENABLE_DYNAMIC_HDR10
>  m_hdr10plus_api = hdr10plus_api_get();
> -numCimInfo = 0;
> -cim = NULL;
> +m_numCimInfo = 0;
> +m_cim = NULL;
>  #endif
>
>  m_prevTonemapPayload.payload = NULL;
> @@ -403,6 +403,11 @@
>  m_nalList.m_annexB = !!m_param->bAnnexB;
>
>  m_emitCLLSEI = p->maxCLL || p->maxFALL;
> +
> +#if ENABLE_DYNAMIC_HDR10
> +if (m_bToneMap)
> +m_numCimInfo = m_hdr10plus_api->hdr10plus_
> json_to_movie_cim(m_param->toneMapFile, m_cim);
> +#endif
>  }
>
>  void Encoder::stopJobs()
> @@ -434,7 +439,8 @@
>  void Encoder::destroy()
>  {
>  #if ENABLE_DYNAMIC_HDR10
> -m_hdr10plus_api->hdr10plus_clear_movie(cim, numCimInfo);
> +if (m_bToneMap)
> +m_hdr10plus_api->hdr10plus_clear_movie(m_cim, m_numCimInfo);
>  #endif
>
>  if (m_exportedPic)
> @@ -610,19 +616,18 @@
>  #if ENABLE_DYNAMIC_HDR10
>  if (m_bToneMap)
>  {
> -if (pic_in->poc == 0)
> -numCimInfo = m_hdr10plus_api->hdr10plus_
> json_to_movie_cim(m_param->toneMapFile, cim);
> -if (pic_in->poc < numCimInfo)
> +int currentPOC = m_pocLast + 1;
> +if (currentPOC < m_numCimInfo)
>  {
>  int32_t i = 0;
>  toneMap.payloadSize = 0;
> -while (cim[pic_in->poc][i] == 0xFF)
> -toneMap.payloadSize += cim[pic_in->poc][i++] + 1;
> -toneMap.payloadSize += cim[pic_in->poc][i] + 1;
> +while (m_cim[currentPOC][i] == 0xFF)
> +toneMap.payloadSize += m_cim[currentPOC][i++] + 1;
> +toneMap.payloadSize += m_cim[currentPOC][i] + 1;
>
>  toneMap.payload = (uint8_t*)x265_malloc(sizeof(uint8_t)
> * toneMap.payloadSize);
>  toneMap.payloadType = USER_DATA_REGISTERED_ITU_T_T35;
> -memcpy(toneMap.payload, cim[pic_in->poc],
> toneMap.payloadSize);
> +memcpy(toneMap.payload, m_cim[currentPOC],
> toneMap.payloadSize);
>  }
>  }
>  #endif
> @@ -716,7 +721,12 @@
>
>  if (inFrame->m_userSEI.numPayloads)
>  {
> -inFrame->m_userSEI.payloads = new
> x265_sei_payload[numPayloads];
> +if (!inFrame->m_userSEI.payloads)
> +{
> +inFrame->m_userSEI.payloads = new
> x265_sei_payload[numPayloads];
> +for (int i = 0; i < numPayloads; i++)
> +inFrame->m_userSEI.payloads[i].payload = NULL;
> +}
>  for (int i = 0; i < numPayloads; i++)
>  {
>  x265_sei_payload input;
> @@ -726,7 +736,8 @@
>  input = pic_in->userSEI.payloads[i];
>  int size = inFrame->m_userSEI.payloads[i].payloadSize =
> input.payloadSize;
>  inFrame->m_userSEI.payloads[i].payloadType =
> input.payloadType;
> -inFrame->m_userSEI.payloads[i].payload = new
> uint8_t[size];
> +if (!inFrame->m_userSEI.payloads[i].payload)
> +inFrame->m_userSEI.payloads[i].payload = new
> uint8_t[size];
>  

[x265] [PATCH] Fix memory leak in hdr10plus

2017-07-23 Thread bhavna
# HG changeset patch
# User Bhavna Hariharan 
# Date 1500874610 -19800
#  Mon Jul 24 11:06:50 2017 +0530
# Branch stable
# Node ID c28a95a9ebbf5bf6bb5c9a357fc11e3c3bdea35c
# Parent  bce945545c241ce4bb87d56d283ac8226d862ed5
Fix memory leak in hdr10plus

Rename numCimInfo and cim to m_numCimInfo and m_cim respectively.

Fixes memory leaks cause due to incorrect handling of m_cim and m_userSEI
memory allocation.

Handle crash when empty json file is parsed.

diff -r bce945545c24 -r c28a95a9ebbf source/dynamicHDR10/JsonHelper.cpp
--- a/source/dynamicHDR10/JsonHelper.cppWed Jun 07 16:29:15 2017 +0530
+++ b/source/dynamicHDR10/JsonHelper.cppMon Jul 24 11:06:50 2017 +0530
@@ -188,9 +188,15 @@
 
 tfile.close();
 
-size_t beginning = json_str2.find_first_of("[");
-int fixchar = json_str2[json_str2.size() - 2] == ']' ? 1 : 0;
-return Json::parse(json_str2.substr(beginning,json_str2.size() - 
fixchar),err).array_items();
+vector data;
+if (json_str2.size() != 0)
+{
+size_t beginning = json_str2.find_first_of("[");
+int fixchar = json_str2[json_str2.size() - 2] == ']' ? 1 : 0;
+return Json::parse(json_str2.substr(beginning, json_str2.size() - 
fixchar), err).array_items();
+}
+else
+return data;
 }
 
 bool JsonHelper::validatePathExtension(string )
diff -r bce945545c24 -r c28a95a9ebbf source/encoder/encoder.cpp
--- a/source/encoder/encoder.cppWed Jun 07 16:29:15 2017 +0530
+++ b/source/encoder/encoder.cppMon Jul 24 11:06:50 2017 +0530
@@ -88,8 +88,8 @@
 
 #if ENABLE_DYNAMIC_HDR10
 m_hdr10plus_api = hdr10plus_api_get();
-numCimInfo = 0;
-cim = NULL;
+m_numCimInfo = 0;
+m_cim = NULL;
 #endif
 
 m_prevTonemapPayload.payload = NULL;
@@ -403,6 +403,11 @@
 m_nalList.m_annexB = !!m_param->bAnnexB;
 
 m_emitCLLSEI = p->maxCLL || p->maxFALL;
+
+#if ENABLE_DYNAMIC_HDR10
+if (m_bToneMap)
+m_numCimInfo = 
m_hdr10plus_api->hdr10plus_json_to_movie_cim(m_param->toneMapFile, m_cim);
+#endif
 }
 
 void Encoder::stopJobs()
@@ -434,7 +439,8 @@
 void Encoder::destroy()
 {
 #if ENABLE_DYNAMIC_HDR10
-m_hdr10plus_api->hdr10plus_clear_movie(cim, numCimInfo);
+if (m_bToneMap)
+m_hdr10plus_api->hdr10plus_clear_movie(m_cim, m_numCimInfo);
 #endif
 
 if (m_exportedPic)
@@ -610,19 +616,18 @@
 #if ENABLE_DYNAMIC_HDR10
 if (m_bToneMap)
 {
-if (pic_in->poc == 0)
-numCimInfo = 
m_hdr10plus_api->hdr10plus_json_to_movie_cim(m_param->toneMapFile, cim);
-if (pic_in->poc < numCimInfo)
+int currentPOC = m_pocLast + 1;
+if (currentPOC < m_numCimInfo)
 {
 int32_t i = 0;
 toneMap.payloadSize = 0;
-while (cim[pic_in->poc][i] == 0xFF)
-toneMap.payloadSize += cim[pic_in->poc][i++] + 1;
-toneMap.payloadSize += cim[pic_in->poc][i] + 1;
+while (m_cim[currentPOC][i] == 0xFF)
+toneMap.payloadSize += m_cim[currentPOC][i++] + 1;
+toneMap.payloadSize += m_cim[currentPOC][i] + 1;
 
 toneMap.payload = (uint8_t*)x265_malloc(sizeof(uint8_t) * 
toneMap.payloadSize);
 toneMap.payloadType = USER_DATA_REGISTERED_ITU_T_T35;
-memcpy(toneMap.payload, cim[pic_in->poc], toneMap.payloadSize);
+memcpy(toneMap.payload, m_cim[currentPOC], 
toneMap.payloadSize);
 }
 }
 #endif
@@ -716,7 +721,12 @@
 
 if (inFrame->m_userSEI.numPayloads)
 {
-inFrame->m_userSEI.payloads = new x265_sei_payload[numPayloads];
+if (!inFrame->m_userSEI.payloads)
+{
+inFrame->m_userSEI.payloads = new 
x265_sei_payload[numPayloads];
+for (int i = 0; i < numPayloads; i++)
+inFrame->m_userSEI.payloads[i].payload = NULL;
+}
 for (int i = 0; i < numPayloads; i++)
 {
 x265_sei_payload input;
@@ -726,7 +736,8 @@
 input = pic_in->userSEI.payloads[i];
 int size = inFrame->m_userSEI.payloads[i].payloadSize = 
input.payloadSize;
 inFrame->m_userSEI.payloads[i].payloadType = input.payloadType;
-inFrame->m_userSEI.payloads[i].payload = new uint8_t[size];
+if (!inFrame->m_userSEI.payloads[i].payload)
+inFrame->m_userSEI.payloads[i].payload = new uint8_t[size];
 memcpy(inFrame->m_userSEI.payloads[i].payload, input.payload, 
size);
 }
 if (toneMap.payload)
diff -r bce945545c24 -r c28a95a9ebbf source/encoder/encoder.h
--- a/source/encoder/encoder.h  Wed Jun 07 16:29:15 2017 +0530
+++ b/source/encoder/encoder.h  Mon Jul 24 11:06:50 2017 +0530
@@ -178,8 +178,8 @@
 
 #ifdef ENABLE_DYNAMIC_HDR10
 const hdr10plus_api