Boris Nagels

2018-01-02 22:31:08 UTC

Calling x264_encoder_close() will also call x264_analyse_free_costs() to free up the allocated resources. The current implementation checks the wrong pointers if they're valid. This is because the pointers that are stored in the x264_t structure are shifted (see implementation for x264_analyse_init_costs()). This patch also stores the original pointers that were allocated, to be able to free the memory when the codec is closed. In the original implementation, this leads to memory-leaks and/or memory corruption when the encoder is opened/closed and reopened.

---

common/common.h | 2 ++

encoder/analyse.c | 21 ++++++++++++---------

2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/common/common.h b/common/common.h

index 867b2073..b292fcdc 100644

--- a/common/common.h

+++ b/common/common.h

@@ -563,6 +563,8 @@ struct x264_t

/* mv/ref cost arrays. */

uint16_t *cost_mv[QP_MAX+1];

uint16_t *cost_mv_fpel[QP_MAX+1][4];

+ uint16_t *p_cost_mv[QP_MAX+1];

+ uint16_t *p_cost_mv_fpel[QP_MAX+1][4];

const uint8_t *chroma_qp_table; /* includes both the nonlinear luma->chroma mapping and chroma_qp_offset */

diff --git a/encoder/analyse.c b/encoder/analyse.c

index 036d6c15..07668a63 100644

--- a/encoder/analyse.c

+++ b/encoder/analyse.c

@@ -270,8 +270,8 @@ static int init_costs( x264_t *h, float *logs, int qp )

int mv_range = h->param.analyse.i_mv_range;

int lambda = x264_lambda_tab[qp];

/* factor of 4 from qpel, 2 from sign, and 2 because mv can be opposite from mvp */

- CHECKED_MALLOC( h->cost_mv[qp], (4*4*mv_range + 1) * sizeof(uint16_t) );

- h->cost_mv[qp] += 2*4*mv_range;

+ CHECKED_MALLOC( h->p_cost_mv[qp], (4*4*mv_range + 1) * sizeof(uint16_t) );

+ h->cost_mv[qp] = h->p_cost_mv[qp] + 2*4*mv_range;

for( int i = 0; i <= 2*4*mv_range; i++ )

{

h->cost_mv[qp][-i] =

@@ -286,8 +286,8 @@ static int init_costs( x264_t *h, float *logs, int qp )

{

for( int j = 0; j < 4; j++ )

{

- CHECKED_MALLOC( h->cost_mv_fpel[qp][j], (4*mv_range + 1) * sizeof(uint16_t) );

- h->cost_mv_fpel[qp][j] += 2*mv_range;

+ CHECKED_MALLOC( h->p_cost_mv_fpel[qp][j], (4*mv_range + 1) * sizeof(uint16_t) );

+ h->cost_mv_fpel[qp][j] = h->p_cost_mv_fpel[qp][j] + 2*mv_range;

for( int i = -2*mv_range; i < 2*mv_range; i++ )

h->cost_mv_fpel[qp][j][i] = h->cost_mv[qp][i*4+j];

}

@@ -330,11 +330,14 @@ void x264_analyse_free_costs( x264_t *h )

int mv_range = h->param.analyse.i_mv_range;

for( int i = 0; i < QP_MAX+1; i++ )

{

- if( h->cost_mv[i] )

- x264_free( h->cost_mv[i] - 2*4*mv_range );

- if( h->cost_mv_fpel[i][0] )

- for( int j = 0; j < 4; j++ )

- x264_free( h->cost_mv_fpel[i][j] - 2*mv_range );

+ if (h->p_cost_mv[i]) {

+ x264_free(h->p_cost_mv[i]);

+ }

+ if(h->p_cost_mv_fpel[i][0]) {

+ for( int j = 0; j < 4; j++ ) {

+ x264_free(h->p_cost_mv_fpel[i][j]);

+ }

+ }

}

}

---

common/common.h | 2 ++

encoder/analyse.c | 21 ++++++++++++---------

2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/common/common.h b/common/common.h

index 867b2073..b292fcdc 100644

--- a/common/common.h

+++ b/common/common.h

@@ -563,6 +563,8 @@ struct x264_t

/* mv/ref cost arrays. */

uint16_t *cost_mv[QP_MAX+1];

uint16_t *cost_mv_fpel[QP_MAX+1][4];

+ uint16_t *p_cost_mv[QP_MAX+1];

+ uint16_t *p_cost_mv_fpel[QP_MAX+1][4];

const uint8_t *chroma_qp_table; /* includes both the nonlinear luma->chroma mapping and chroma_qp_offset */

diff --git a/encoder/analyse.c b/encoder/analyse.c

index 036d6c15..07668a63 100644

--- a/encoder/analyse.c

+++ b/encoder/analyse.c

@@ -270,8 +270,8 @@ static int init_costs( x264_t *h, float *logs, int qp )

int mv_range = h->param.analyse.i_mv_range;

int lambda = x264_lambda_tab[qp];

/* factor of 4 from qpel, 2 from sign, and 2 because mv can be opposite from mvp */

- CHECKED_MALLOC( h->cost_mv[qp], (4*4*mv_range + 1) * sizeof(uint16_t) );

- h->cost_mv[qp] += 2*4*mv_range;

+ CHECKED_MALLOC( h->p_cost_mv[qp], (4*4*mv_range + 1) * sizeof(uint16_t) );

+ h->cost_mv[qp] = h->p_cost_mv[qp] + 2*4*mv_range;

for( int i = 0; i <= 2*4*mv_range; i++ )

{

h->cost_mv[qp][-i] =

@@ -286,8 +286,8 @@ static int init_costs( x264_t *h, float *logs, int qp )

{

for( int j = 0; j < 4; j++ )

{

- CHECKED_MALLOC( h->cost_mv_fpel[qp][j], (4*mv_range + 1) * sizeof(uint16_t) );

- h->cost_mv_fpel[qp][j] += 2*mv_range;

+ CHECKED_MALLOC( h->p_cost_mv_fpel[qp][j], (4*mv_range + 1) * sizeof(uint16_t) );

+ h->cost_mv_fpel[qp][j] = h->p_cost_mv_fpel[qp][j] + 2*mv_range;

for( int i = -2*mv_range; i < 2*mv_range; i++ )

h->cost_mv_fpel[qp][j][i] = h->cost_mv[qp][i*4+j];

}

@@ -330,11 +330,14 @@ void x264_analyse_free_costs( x264_t *h )

int mv_range = h->param.analyse.i_mv_range;

for( int i = 0; i < QP_MAX+1; i++ )

{

- if( h->cost_mv[i] )

- x264_free( h->cost_mv[i] - 2*4*mv_range );

- if( h->cost_mv_fpel[i][0] )

- for( int j = 0; j < 4; j++ )

- x264_free( h->cost_mv_fpel[i][j] - 2*mv_range );

+ if (h->p_cost_mv[i]) {

+ x264_free(h->p_cost_mv[i]);

+ }

+ if(h->p_cost_mv_fpel[i][0]) {

+ for( int j = 0; j < 4; j++ ) {

+ x264_free(h->p_cost_mv_fpel[i][j]);

+ }

+ }

}

}

--

2.14.3 (Apple Git-98)

2.14.3 (Apple Git-98)