From 4b46c74536b701844a997d87bf21dfd66b8c0e08 Mon Sep 17 00:00:00 2001 From: Venkatarama Avadhani Date: Thu, 28 Dec 2017 16:57:40 +0530 Subject: Fixing Underflow of ps_dec->u2_num_mbs_left In multi-thread decode, the decoder would try to decode without dequeueing a job in case the next slice indicated that it belongs to the same row as being decoded currently. In single thread case, there was a check to ensure that the decoder does not continue when there are no MBs left. Adding a similar check for multi-thread decode as well. Bug: 69269702 Test: manual Change-Id: Ibbe5202dbb270625e4f592b4fdb8ef0ec71a979d (cherry picked from commit 00a2482c8dfa3550bcbfa515a93a4cead5daf8e9) --- decoder/impeg2d_dec_hdr.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/decoder/impeg2d_dec_hdr.c b/decoder/impeg2d_dec_hdr.c index 7a4d522..c16bfe2 100644 --- a/decoder/impeg2d_dec_hdr.c +++ b/decoder/impeg2d_dec_hdr.c @@ -1018,23 +1018,30 @@ void impeg2d_dec_pic_data_thread(dec_state_t *ps_dec) if(i4_continue_decode) { - /* If the slice is from the same row, then continue decoding without dequeue */ - if((temp - 1) == i4_cur_row) + if (0 != ps_dec->u2_num_mbs_left) { - i4_dequeue_job = 0; - break; - } - - if(temp < ps_dec->i4_end_mb_y) - { - i4_cur_row = ps_dec->u2_mb_y; + /* If the slice is from the same row, then continue decoding without dequeue */ + if((temp - 1) == i4_cur_row) + { + i4_dequeue_job = 0; + } + else + { + if(temp < ps_dec->i4_end_mb_y) + { + i4_cur_row = ps_dec->u2_mb_y; + } + else + { + i4_dequeue_job = 1; + } + } } else { i4_dequeue_job = 1; } break; - } else break; -- cgit v1.2.3 From 89616f8c127f94c78e9614658f6e367a1ffa1ddd Mon Sep 17 00:00:00 2001 From: Venkatarama Avadhani Date: Mon, 18 Dec 2017 15:57:18 +0530 Subject: Correcting Buffer Allocation for Shared Display In case of shared display mode, the picture buffer allocated has to be larger to accomodate the incorrect half pel reference to the last row in the picture. Adding memory for the same. Bug: 70350015 Bug: 70349694 Bug: 70349612 Bug: 70349754 Bug: 70349868 Bug: 70526352 Bug: 70350086 Test: manual Change-Id: I41905d101093ae20ab14193c21669b8c4a24f30c (cherry picked from commit 5c1ed47b1fc1a0f763c251d62db14f3d74cb3141) --- decoder/impeg2d_api_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/decoder/impeg2d_api_main.c b/decoder/impeg2d_api_main.c index 606a5dc..51f7047 100644 --- a/decoder/impeg2d_api_main.c +++ b/decoder/impeg2d_api_main.c @@ -824,7 +824,8 @@ IV_API_CALL_STATUS_T impeg2d_api_get_buf_info(iv_obj_t *ps_dechdl, { u4_stride = ps_dec_state->u4_frm_buf_stride; } - u4_height = ((ps_dec_state->u2_frame_height + 15) >> 4) << 4; + u4_stride = ALIGN16(u4_stride); + u4_height = ALIGN32(ps_dec_state->u2_frame_height) + 9; if(ps_dec_state->i4_chromaFormat == IV_YUV_420P) { -- cgit v1.2.3 From 8f798b99bebead013f7fc78029509bbea528c748 Mon Sep 17 00:00:00 2001 From: Venkatarama Avadhani Date: Wed, 9 Aug 2017 12:15:02 +0530 Subject: Adding Error Check for Output Buffer Size The output buffer size given by the application, needs to be checked in every process call. This is required in the case of resolution change. Bug: 70399408 Test: manual Change-Id: Id0d615e44d30f61702b3839be7b679d7d77a2411 (cherry picked from commit 7c88212153fbff998b337e899b496e2e382af54c) --- decoder/impeg2d_api_main.c | 199 +++++++++++++++++++++++++++++++++------------ decoder/impeg2d_structs.h | 2 + test/decoder/main.c | 7 ++ 3 files changed, 154 insertions(+), 54 deletions(-) diff --git a/decoder/impeg2d_api_main.c b/decoder/impeg2d_api_main.c index 51f7047..e3c83bf 100644 --- a/decoder/impeg2d_api_main.c +++ b/decoder/impeg2d_api_main.c @@ -125,6 +125,7 @@ void impeg2d_init_arch(void *pv_codec); void impeg2d_init_function_ptr(void *pv_codec); +UWORD32 impeg2d_get_outbuf_size(WORD32 pic_wd,UWORD32 pic_ht, WORD32 u1_chroma_format,UWORD32 *p_buf_size); /*****************************************************************************/ /* */ @@ -211,6 +212,8 @@ IV_API_CALL_STATUS_T impeg2d_api_set_display_frame(iv_obj_t *ps_dechdl, dec_state_t *ps_dec_state; dec_state_multi_core_t *ps_dec_state_multi_core; UWORD32 u4_num_disp_bufs; + UWORD32 u4_disp_buf_size[3]; + UWORD32 num_bufs; dec_disp_ip = (ivd_set_display_frame_ip_t *)pv_api_ip; @@ -224,12 +227,41 @@ IV_API_CALL_STATUS_T impeg2d_api_set_display_frame(iv_obj_t *ps_dechdl, ps_dec_state_multi_core = (dec_state_multi_core_t *) (ps_dechdl->pv_codec_handle); ps_dec_state = ps_dec_state_multi_core->ps_dec_state[0]; + num_bufs = dec_disp_ip->s_disp_buffer[0].u4_num_bufs; + if(ps_dec_state->u4_share_disp_buf) { pic_buf_t *ps_pic_buf; ps_pic_buf = (pic_buf_t *)ps_dec_state->pv_pic_buf_base; + + /* Get the sizes of the first buffer structure. Compare this with the + * rest to make sure all the buffers are of the same size. + */ + u4_disp_buf_size[0] = + dec_disp_ip->s_disp_buffer[0].u4_min_out_buf_size[0]; + u4_disp_buf_size[1] = + dec_disp_ip->s_disp_buffer[0].u4_min_out_buf_size[1]; + u4_disp_buf_size[2] = + dec_disp_ip->s_disp_buffer[0].u4_min_out_buf_size[2]; for(i = 0; i < u4_num_disp_bufs; i++) { + /* Verify all buffer structures have the same sized buffers as the + * first buffer structure*/ + if ((dec_disp_ip->s_disp_buffer[i].u4_min_out_buf_size[0] + != u4_disp_buf_size[0]) + || (dec_disp_ip->s_disp_buffer[i].u4_min_out_buf_size[1] + != u4_disp_buf_size[1]) + || (dec_disp_ip->s_disp_buffer[i].u4_min_out_buf_size[2] + != u4_disp_buf_size[2])) + { + return IV_FAIL; + } + /* Verify all buffer structures have the same number of + * buffers (e.g. y, u, v) */ + if (dec_disp_ip->s_disp_buffer[i].u4_num_bufs != num_bufs) + { + return IV_FAIL; + } ps_pic_buf->pu1_y = dec_disp_ip->s_disp_buffer[i].pu1_bufs[0]; if(IV_YUV_420P == ps_dec_state->i4_chromaFormat) @@ -764,35 +796,6 @@ IV_API_CALL_STATUS_T impeg2d_api_get_buf_info(iv_obj_t *ps_dechdl, ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_in_bufs = 1; ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_out_bufs = 1; - if(ps_dec_state->i4_chromaFormat == IV_YUV_420P) - { - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_out_bufs = - MIN_OUT_BUFS_420; - } - else if((ps_dec_state->i4_chromaFormat == IV_YUV_420SP_UV) - || (ps_dec_state->i4_chromaFormat == IV_YUV_420SP_VU)) - { - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_out_bufs = - MIN_OUT_BUFS_420SP; - } - else if(ps_dec_state->i4_chromaFormat == IV_YUV_422ILE) - { - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_out_bufs = - MIN_OUT_BUFS_422ILE; - } - else if(ps_dec_state->i4_chromaFormat == IV_RGB_565) - { - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_out_bufs = - MIN_OUT_BUFS_RGB565; - } - else - { - //Invalid chroma format; Error code may be updated, verify in testing if needed - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_error_code = - IVD_INIT_DEC_COL_FMT_NOT_SUPPORTED; - return IV_FAIL; - } - for(u4_i = 0; u4_i < IVD_VIDDEC_MAX_IO_BUFFERS; u4_i++) { ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_in_buf_size[u4_i] = @@ -827,31 +830,19 @@ IV_API_CALL_STATUS_T impeg2d_api_get_buf_info(iv_obj_t *ps_dechdl, u4_stride = ALIGN16(u4_stride); u4_height = ALIGN32(ps_dec_state->u2_frame_height) + 9; - if(ps_dec_state->i4_chromaFormat == IV_YUV_420P) - { - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[0] = - (u4_stride * u4_height); - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[1] = - (u4_stride * u4_height) >> 2; - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[2] = - (u4_stride * u4_height) >> 2; - } - else if((ps_dec_state->i4_chromaFormat == IV_YUV_420SP_UV) - || (ps_dec_state->i4_chromaFormat == IV_YUV_420SP_VU)) - { - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[0] = - (u4_stride * u4_height); - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[1] = - (u4_stride * u4_height) >> 1; - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[2] = 0; - } - else if(ps_dec_state->i4_chromaFormat == IV_YUV_422ILE) + ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_out_bufs = + impeg2d_get_outbuf_size( + u4_stride, + u4_height, + ps_dec_state->i4_chromaFormat, + &ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[0]); + + if (0 == ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_num_out_bufs) { - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[0] = - (u4_stride * u4_height) * 2; - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[1] = - ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_min_out_buf_size[2] = - 0; + //Invalid chroma format; Error code may be updated, verify in testing if needed + ps_ctl_bufinfo_op->s_ivd_ctl_getbufinfo_op_t.u4_error_code = + IVD_INIT_DEC_COL_FMT_NOT_SUPPORTED; + return IV_FAIL; } /* Adding initialization for 2 uninitialized values */ @@ -1514,6 +1505,100 @@ IV_API_CALL_STATUS_T impeg2d_api_fill_mem_rec(void *pv_api_ip,void *pv_api_op) } +UWORD32 impeg2d_get_outbuf_size(WORD32 pic_wd,UWORD32 pic_ht, WORD32 u1_chroma_format,UWORD32 *p_buf_size) +{ + UWORD32 u4_min_num_out_bufs = 0; + if(u1_chroma_format == IV_YUV_420P) + u4_min_num_out_bufs = MIN_OUT_BUFS_420; + else if(u1_chroma_format == IV_YUV_422ILE) + u4_min_num_out_bufs = MIN_OUT_BUFS_422ILE; + else if(u1_chroma_format == IV_RGB_565) + u4_min_num_out_bufs = MIN_OUT_BUFS_RGB565; + else if((u1_chroma_format == IV_YUV_420SP_UV) + || (u1_chroma_format == IV_YUV_420SP_VU)) + u4_min_num_out_bufs = MIN_OUT_BUFS_420SP; + + if(u1_chroma_format == IV_YUV_420P) + { + p_buf_size[0] = (pic_wd * pic_ht); + p_buf_size[1] = (pic_wd * pic_ht) + >> 2; + p_buf_size[2] = (pic_wd * pic_ht) + >> 2; + } + else if(u1_chroma_format == IV_YUV_422ILE) + { + p_buf_size[0] = (pic_wd * pic_ht) + * 2; + p_buf_size[1] = + p_buf_size[2] = 0; + } + else if(u1_chroma_format == IV_RGB_565) + { + p_buf_size[0] = (pic_wd * pic_ht) + * 2; + p_buf_size[1] = + p_buf_size[2] = 0; + } + else if((u1_chroma_format == IV_YUV_420SP_UV) + || (u1_chroma_format == IV_YUV_420SP_VU)) + { + p_buf_size[0] = (pic_wd * pic_ht); + p_buf_size[1] = (pic_wd * pic_ht) + >> 1; + p_buf_size[2] = 0; + } + + return u4_min_num_out_bufs; +} + +WORD32 check_app_out_buf_size(dec_state_t *ps_dec) +{ + UWORD32 au4_min_out_buf_size[IVD_VIDDEC_MAX_IO_BUFFERS]; + UWORD32 u4_min_num_out_bufs, i; + UWORD32 pic_wd, pic_ht; + + pic_wd = ps_dec->u2_horizontal_size; + pic_ht = ps_dec->u2_vertical_size; + + if(ps_dec->u4_frm_buf_stride > pic_wd) + pic_wd = ps_dec->u4_frm_buf_stride; + + u4_min_num_out_bufs = impeg2d_get_outbuf_size(pic_wd, pic_ht, ps_dec->i4_chromaFormat, &au4_min_out_buf_size[0]); + + if (0 == ps_dec->u4_share_disp_buf) + { + if(ps_dec->ps_out_buf->u4_num_bufs < u4_min_num_out_bufs) + { + return IV_FAIL; + } + + for (i = 0 ; i < u4_min_num_out_bufs; i++) + { + if(ps_dec->ps_out_buf->u4_min_out_buf_size[i] < au4_min_out_buf_size[i]) + return (IV_FAIL); + } + } + else + { + if(ps_dec->as_disp_buffers[0].u4_num_bufs < u4_min_num_out_bufs) + return IV_FAIL; + + for (i = 0 ; i < u4_min_num_out_bufs; i++) + { + /* We need to check only with the disp_buffer[0], because we have + * already ensured that all the buffers are of the same size in + * impeg2d_api_set_display_frame. + */ + if(ps_dec->as_disp_buffers[0].u4_min_out_buf_size[i] < + au4_min_out_buf_size[i]) + return (IV_FAIL); + } + } + + return(IV_SUCCESS); +} + /*****************************************************************************/ @@ -1797,7 +1882,6 @@ IV_API_CALL_STATUS_T impeg2d_api_init(iv_obj_t *ps_dechdl, } } - ps_dec_state = ps_dec_state_multi_core->ps_dec_state[0]; if((ps_dec_state->i4_chromaFormat == IV_YUV_422ILE) @@ -3382,6 +3466,13 @@ IV_API_CALL_STATUS_T impeg2d_api_entity(iv_obj_t *ps_dechdl, { ps_dec_state->u1_flushcnt = 0; + ps_dec_state->ps_out_buf = &ps_dec_ip->s_ivd_video_decode_ip_t.s_out_buffer; + if (IV_SUCCESS != check_app_out_buf_size(ps_dec_state)) + { + ps_dec_op->s_ivd_video_decode_op_t.u4_error_code = IVD_DISP_FRM_ZERO_OP_BUF_SIZE; + return IV_FAIL; + } + /*************************************************************************/ /* Frame Decode */ /*************************************************************************/ diff --git a/decoder/impeg2d_structs.h b/decoder/impeg2d_structs.h index 46538bf..bdda8b8 100644 --- a/decoder/impeg2d_structs.h +++ b/decoder/impeg2d_structs.h @@ -363,6 +363,8 @@ typedef struct dec_state_struct_t /* Number of bytes in the input bitstream */ UWORD32 u4_num_inp_bytes; + ivd_out_bufdesc_t *ps_out_buf; + /* Bytes consumed */ WORD32 i4_bytes_consumed; diff --git a/test/decoder/main.c b/test/decoder/main.c index cfce433..9502dec 100644 --- a/test/decoder/main.c +++ b/test/decoder/main.c @@ -2246,6 +2246,7 @@ int main(WORD32 argc, CHAR *argv[]) u4_ip_buf_len); codec_exit(ac_error_str); } + s_app_ctx.num_disp_buf = s_ctl_op.u4_num_disp_bufs; /* Allocate output buffer only if display buffers are not shared */ /* Or if shared and output is 420P */ @@ -3016,6 +3017,12 @@ int main(WORD32 argc, CHAR *argv[]) break; } + else if (IVD_DISP_FRM_ZERO_OP_BUF_SIZE == + (IMPEG2D_ERROR_CODES_T)s_video_decode_op.u4_error_code) + { + printf("Output buffer is not large enough!\n"); + break; + } } if((1 == s_app_ctx.display) && -- cgit v1.2.3