From 7bc9d4d29a00f38f9fd427004f59ae3a8533b930 Mon Sep 17 00:00:00 2001 From: Hamsalekha S Date: Wed, 10 Jun 2020 12:16:40 +0530 Subject: [PATCH 01/17] Decoder: Fix heap buffer overflow. Fix bitstream buffer overflow in the function ih264d_parse_sei_message Bug: 152895390 Test: POC in bug Change-Id: I41ff1f7b2834c2d09e546b8e3d37e4cd4abfa28d --- decoder/ih264d_bitstrm.h | 9 ++++++--- decoder/ih264d_cabac.c | 2 +- decoder/ih264d_parse_headers.c | 4 ++-- decoder/ih264d_sei.c | 20 ++++++++++++++++---- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/decoder/ih264d_bitstrm.h b/decoder/ih264d_bitstrm.h index 49cd5e7..8bb06fb 100644 --- a/decoder/ih264d_bitstrm.h +++ b/decoder/ih264d_bitstrm.h @@ -57,7 +57,7 @@ typedef struct { UWORD32 u4_ofst; /* Offset in the buffer for the current bit */ UWORD32 *pu4_buffer; /* Bitstream Buffer */ - UWORD32 u4_max_ofst; /* Position of the last bit read in the current buffer */ + UWORD32 u4_max_ofst; /* points to first bit beyond the buffer */ void * pv_codec_handle; /* For Error Handling */ } dec_bit_stream_t; @@ -88,10 +88,13 @@ WORD32 ih264d_flush_bits_h264(dec_bit_stream_t *, WORD32); ************************************************************************** */ -#define MORE_RBSP_DATA(ps_bitstrm) \ - (ps_bitstrm->u4_ofst < ps_bitstrm->u4_max_ofst) + #define EXCEED_OFFSET(ps_bitstrm) \ (ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst) +#define CHECK_BITS_SUFFICIENT(ps_bitstrm, bits_to_read) \ + (ps_bitstrm->u4_ofst + bits_to_read <= ps_bitstrm->u4_max_ofst) +#define MORE_RBSP_DATA(ps_bitstrm) \ + CHECK_BITS_SUFFICIENT(ps_bitstrm, 1) void GoToByteBoundary(dec_bit_stream_t * ps_bitstrm); UWORD8 ih264d_check_byte_aligned(dec_bit_stream_t * ps_bitstrm); diff --git a/decoder/ih264d_cabac.c b/decoder/ih264d_cabac.c index 38028ae..ef1fafc 100644 --- a/decoder/ih264d_cabac.c +++ b/decoder/ih264d_cabac.c @@ -69,7 +69,7 @@ WORD32 ih264d_init_cabac_dec_envirnoment(decoding_envirnoment_t * ps_cab_env, 32); FLUSHBITS(ps_bitstrm->u4_ofst, 9) - if(ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst) + if(EXCEED_OFFSET(ps_bitstrm)) return ERROR_EOB_FLUSHBITS_T; ps_cab_env->u4_code_int_val_ofst = u4_code_int_val_ofst; diff --git a/decoder/ih264d_parse_headers.c b/decoder/ih264d_parse_headers.c index f286e29..9220707 100644 --- a/decoder/ih264d_parse_headers.c +++ b/decoder/ih264d_parse_headers.c @@ -463,7 +463,7 @@ WORD32 ih264d_parse_pps(dec_struct_t * ps_dec, dec_bit_stream_t * ps_bitstrm) /* In case bitstream read has exceeded the filled size, then return an error */ - if(ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst + 8) + if(EXCEED_OFFSET(ps_bitstrm)) { return ERROR_INV_SPS_PPS_T; } @@ -1093,7 +1093,7 @@ WORD32 ih264d_parse_sps(dec_struct_t *ps_dec, dec_bit_stream_t *ps_bitstrm) /* In case bitstream read has exceeded the filled size, then return an error */ - if (ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst) + if (EXCEED_OFFSET(ps_bitstrm)) { return ERROR_INV_SPS_PPS_T; } diff --git a/decoder/ih264d_sei.c b/decoder/ih264d_sei.c index 4375671..ac4d056 100644 --- a/decoder/ih264d_sei.c +++ b/decoder/ih264d_sei.c @@ -759,8 +759,12 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, { ui4_payload_type = 0; + if(!CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) + { + return ERROR_EOB_GETBITS_T; + } u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); - while(0xff == u4_bits && !EXCEED_OFFSET(ps_bitstrm)) + while(0xff == u4_bits && CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) { u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); ui4_payload_type += 255; @@ -768,14 +772,22 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, ui4_payload_type += u4_bits; ui4_payload_size = 0; + if(!CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) + { + return ERROR_EOB_GETBITS_T; + } u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); - while(0xff == u4_bits && !EXCEED_OFFSET(ps_bitstrm)) + while(0xff == u4_bits && CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) { u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); ui4_payload_size += 255; } ui4_payload_size += u4_bits; + if(!CHECK_BITS_SUFFICIENT(ps_bitstrm, (ui4_payload_size << 3))) + { + return ERROR_EOB_GETBITS_T; + } i4_status = ih264d_parse_sei_payload(ps_bitstrm, ui4_payload_type, ui4_payload_size, ps_dec); if(i4_status != OK) @@ -789,7 +801,7 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, H264_DEC_DEBUG_PRINT("\nError in parsing SEI message"); } while(0 == ih264d_check_byte_aligned(ps_bitstrm) - && !EXCEED_OFFSET(ps_bitstrm)) + && CHECK_BITS_SUFFICIENT(ps_bitstrm, 1)) { u4_bits = ih264d_get_bit_h264(ps_bitstrm); if(u4_bits) @@ -799,7 +811,7 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, } } } - while(ps_bitstrm->u4_ofst < ps_bitstrm->u4_max_ofst); + while(MORE_RBSP_DATA(ps_bitstrm)); return (i4_status); } From 9ebc993dc7f202af88112e2ab974327aed570a85 Mon Sep 17 00:00:00 2001 From: Hamsalekha S Date: Wed, 10 Jun 2020 12:16:40 +0530 Subject: [PATCH 02/17] Decoder: Fix heap buffer overflow. Fix bitstream buffer overflow in the function ih264d_parse_sei_message Bug: 152895390 Test: POC in bug Change-Id: I41ff1f7b2834c2d09e546b8e3d37e4cd4abfa28d --- decoder/ih264d_bitstrm.h | 9 ++++++--- decoder/ih264d_cabac.c | 2 +- decoder/ih264d_parse_headers.c | 4 ++-- decoder/ih264d_sei.c | 20 ++++++++++++++++---- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/decoder/ih264d_bitstrm.h b/decoder/ih264d_bitstrm.h index 49cd5e7..8bb06fb 100644 --- a/decoder/ih264d_bitstrm.h +++ b/decoder/ih264d_bitstrm.h @@ -57,7 +57,7 @@ typedef struct { UWORD32 u4_ofst; /* Offset in the buffer for the current bit */ UWORD32 *pu4_buffer; /* Bitstream Buffer */ - UWORD32 u4_max_ofst; /* Position of the last bit read in the current buffer */ + UWORD32 u4_max_ofst; /* points to first bit beyond the buffer */ void * pv_codec_handle; /* For Error Handling */ } dec_bit_stream_t; @@ -88,10 +88,13 @@ WORD32 ih264d_flush_bits_h264(dec_bit_stream_t *, WORD32); ************************************************************************** */ -#define MORE_RBSP_DATA(ps_bitstrm) \ - (ps_bitstrm->u4_ofst < ps_bitstrm->u4_max_ofst) + #define EXCEED_OFFSET(ps_bitstrm) \ (ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst) +#define CHECK_BITS_SUFFICIENT(ps_bitstrm, bits_to_read) \ + (ps_bitstrm->u4_ofst + bits_to_read <= ps_bitstrm->u4_max_ofst) +#define MORE_RBSP_DATA(ps_bitstrm) \ + CHECK_BITS_SUFFICIENT(ps_bitstrm, 1) void GoToByteBoundary(dec_bit_stream_t * ps_bitstrm); UWORD8 ih264d_check_byte_aligned(dec_bit_stream_t * ps_bitstrm); diff --git a/decoder/ih264d_cabac.c b/decoder/ih264d_cabac.c index 38028ae..ef1fafc 100644 --- a/decoder/ih264d_cabac.c +++ b/decoder/ih264d_cabac.c @@ -69,7 +69,7 @@ WORD32 ih264d_init_cabac_dec_envirnoment(decoding_envirnoment_t * ps_cab_env, 32); FLUSHBITS(ps_bitstrm->u4_ofst, 9) - if(ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst) + if(EXCEED_OFFSET(ps_bitstrm)) return ERROR_EOB_FLUSHBITS_T; ps_cab_env->u4_code_int_val_ofst = u4_code_int_val_ofst; diff --git a/decoder/ih264d_parse_headers.c b/decoder/ih264d_parse_headers.c index f286e29..9220707 100644 --- a/decoder/ih264d_parse_headers.c +++ b/decoder/ih264d_parse_headers.c @@ -463,7 +463,7 @@ WORD32 ih264d_parse_pps(dec_struct_t * ps_dec, dec_bit_stream_t * ps_bitstrm) /* In case bitstream read has exceeded the filled size, then return an error */ - if(ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst + 8) + if(EXCEED_OFFSET(ps_bitstrm)) { return ERROR_INV_SPS_PPS_T; } @@ -1093,7 +1093,7 @@ WORD32 ih264d_parse_sps(dec_struct_t *ps_dec, dec_bit_stream_t *ps_bitstrm) /* In case bitstream read has exceeded the filled size, then return an error */ - if (ps_bitstrm->u4_ofst > ps_bitstrm->u4_max_ofst) + if (EXCEED_OFFSET(ps_bitstrm)) { return ERROR_INV_SPS_PPS_T; } diff --git a/decoder/ih264d_sei.c b/decoder/ih264d_sei.c index 4375671..ac4d056 100644 --- a/decoder/ih264d_sei.c +++ b/decoder/ih264d_sei.c @@ -759,8 +759,12 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, { ui4_payload_type = 0; + if(!CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) + { + return ERROR_EOB_GETBITS_T; + } u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); - while(0xff == u4_bits && !EXCEED_OFFSET(ps_bitstrm)) + while(0xff == u4_bits && CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) { u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); ui4_payload_type += 255; @@ -768,14 +772,22 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, ui4_payload_type += u4_bits; ui4_payload_size = 0; + if(!CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) + { + return ERROR_EOB_GETBITS_T; + } u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); - while(0xff == u4_bits && !EXCEED_OFFSET(ps_bitstrm)) + while(0xff == u4_bits && CHECK_BITS_SUFFICIENT(ps_bitstrm, 8)) { u4_bits = ih264d_get_bits_h264(ps_bitstrm, 8); ui4_payload_size += 255; } ui4_payload_size += u4_bits; + if(!CHECK_BITS_SUFFICIENT(ps_bitstrm, (ui4_payload_size << 3))) + { + return ERROR_EOB_GETBITS_T; + } i4_status = ih264d_parse_sei_payload(ps_bitstrm, ui4_payload_type, ui4_payload_size, ps_dec); if(i4_status != OK) @@ -789,7 +801,7 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, H264_DEC_DEBUG_PRINT("\nError in parsing SEI message"); } while(0 == ih264d_check_byte_aligned(ps_bitstrm) - && !EXCEED_OFFSET(ps_bitstrm)) + && CHECK_BITS_SUFFICIENT(ps_bitstrm, 1)) { u4_bits = ih264d_get_bit_h264(ps_bitstrm); if(u4_bits) @@ -799,7 +811,7 @@ WORD32 ih264d_parse_sei_message(dec_struct_t *ps_dec, } } } - while(ps_bitstrm->u4_ofst < ps_bitstrm->u4_max_ofst); + while(MORE_RBSP_DATA(ps_bitstrm)); return (i4_status); } From 6c225fbf97adead15649459bcea7d790c2bac901 Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Wed, 8 Jul 2020 17:20:27 -0700 Subject: [PATCH 03/17] decoder: Allow stride to be smaller than decode width When cropping is enabled, application can request a stride that is larger than display width but smaller than decode width. Bug: 160397536 Test: stagefright -sS Change-Id: I453b2de0474f3ae4d021084729c33d52fc1090dc --- decoder/ih264d_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decoder/ih264d_api.c b/decoder/ih264d_api.c index 3a73938..3dfcbf2 100644 --- a/decoder/ih264d_api.c +++ b/decoder/ih264d_api.c @@ -3253,7 +3253,7 @@ WORD32 ih264d_set_params(iv_obj_t *dec_hdl, void *pv_api_ip, void *pv_api_op) ret = IV_FAIL; } - if(ps_ctl_ip->u4_disp_wd >= ps_dec->u2_pic_wd) + if(ps_ctl_ip->u4_disp_wd >= ps_dec->u2_disp_width) { ps_dec->u4_app_disp_width = ps_ctl_ip->u4_disp_wd; } From 0f2b25d9b1a5fbbcbd24cecd10c9d08369367e0a Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Wed, 12 Aug 2020 18:59:56 -0700 Subject: [PATCH 04/17] decoder: Update reorder depth to account for display latency Decoder returns output with an additional latency of 2. reorder depth retured is now updated to account for this extra latency Also move reorder_depth initialization to parse_sps() Instead of initializing reorder_depth after decoding first picture, initialize it in parse_sps(). Bug: 163127030 Test: poc in bug Test: atest android.mediav2.cts Merged-In: I94b35b2c5df5c910d0159548b168617946a19cc2 Change-Id: I94b35b2c5df5c910d0159548b168617946a19cc2 --- decoder/ih264d_defs.h | 5 +++++ decoder/ih264d_parse_headers.c | 17 +++++++++++++++++ decoder/ih264d_utils.c | 1 - 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/decoder/ih264d_defs.h b/decoder/ih264d_defs.h index 2758a59..73604f1 100644 --- a/decoder/ih264d_defs.h +++ b/decoder/ih264d_defs.h @@ -45,6 +45,11 @@ #define FMT_CONV_NUM_ROWS 16 +/** Decoder currently has an additional latency of 2 pictures when + * returning output for display + */ +#define DISPLAY_LATENCY 2 + /** Bit manipulation macros */ #define CHECKBIT(a,i) ((a) & (1 << i)) #define CLEARBIT(a,i) ((a) &= ~(1 << i)) diff --git a/decoder/ih264d_parse_headers.c b/decoder/ih264d_parse_headers.c index f286e29..0c3091a 100644 --- a/decoder/ih264d_parse_headers.c +++ b/decoder/ih264d_parse_headers.c @@ -1101,6 +1101,23 @@ WORD32 ih264d_parse_sps(dec_struct_t *ps_dec, dec_bit_stream_t *ps_bitstrm) /*--------------------------------------------------------------------*/ /* All initializations to ps_dec are beyond this point */ /*--------------------------------------------------------------------*/ + { + WORD32 reorder_depth = ih264d_get_dpb_size(ps_seq); + if((1 == ps_seq->u1_vui_parameters_present_flag) && + (1 == ps_seq->s_vui.u1_bitstream_restriction_flag)) + { + reorder_depth = ps_seq->s_vui.u4_num_reorder_frames + 1; + } + + if (reorder_depth > H264_MAX_REF_PICS) + { + return ERROR_INV_SPS_PPS_T; + } + + if(ps_seq->u1_frame_mbs_only_flag != 1) + reorder_depth *= 2; + ps_dec->i4_reorder_depth = reorder_depth + DISPLAY_LATENCY; + } ps_dec->u2_disp_height = i4_cropped_ht; ps_dec->u2_disp_width = i4_cropped_wd; diff --git a/decoder/ih264d_utils.c b/decoder/ih264d_utils.c index 35cd7b9..e7208a5 100644 --- a/decoder/ih264d_utils.c +++ b/decoder/ih264d_utils.c @@ -769,7 +769,6 @@ WORD32 ih264d_init_pic(dec_struct_t *ps_dec, else ps_dec->i4_display_delay = ps_seq->s_vui.u4_num_reorder_frames * 2 + 2; } - ps_dec->i4_reorder_depth = ps_dec->i4_display_delay; if(IVD_DECODE_FRAME_OUT == ps_dec->e_frm_out_mode) ps_dec->i4_display_delay = 0; From 41c6f288c40a20f002f2460e027b96ecb6b792e9 Mon Sep 17 00:00:00 2001 From: Aasaipriya Chandran Date: Wed, 10 Jun 2020 13:42:04 +0530 Subject: [PATCH 05/17] Decoder: Fix Null dereference Add checks to validate long term index and max long term index parsed in MMCO commands. Bug: 140358770 Bug: 140680655 Bug: 148772548 Bug: 152148135 Bug: 152434373 Bug: 152550528 Test: poc in bug Change-Id: I9052ce7721491fdd5fb4807ec33e399cee8c70cf --- decoder/ih264d_dpb_manager.h | 3 ++- decoder/ih264d_dpb_mgr.c | 39 +++++++++++++++++++++++++++++------- decoder/ih264d_utils.c | 3 ++- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/decoder/ih264d_dpb_manager.h b/decoder/ih264d_dpb_manager.h index a9539c8..3bf00b7 100644 --- a/decoder/ih264d_dpb_manager.h +++ b/decoder/ih264d_dpb_manager.h @@ -50,6 +50,7 @@ #define RESET_NONREF_PICTURES 7 #define RESET_ALL_PICTURES 8 +#define NO_LONG_TERM_INDICIES 255 struct field_t { /* picNum of tbe reference field */ @@ -93,7 +94,7 @@ typedef struct struct dpb_info_t as_dpb_info[MAX_REF_BUFS]; /** Physical storage for dpbInfo for ref bufs */ UWORD8 u1_num_st_ref_bufs; /** Number of short term ref. buffers */ UWORD8 u1_num_lt_ref_bufs; /** Number of long term ref. buffer */ - UWORD8 u1_max_lt_pic_idx_plus1; /** Maximum long term pictures - 0 to max_long_term_pic_idx */ + UWORD8 u1_max_lt_frame_idx; /** Maximum long term frame index */ UWORD8 u1_num_gaps; /** Total number of outstanding gaps */ void * pv_codec_handle; /* For Error Handling */ WORD32 i4_max_frm_num; /** Max frame number */ diff --git a/decoder/ih264d_dpb_mgr.c b/decoder/ih264d_dpb_mgr.c index 0b8426b..28c9619 100644 --- a/decoder/ih264d_dpb_mgr.c +++ b/decoder/ih264d_dpb_mgr.c @@ -851,6 +851,8 @@ WORD32 ih264d_ref_idx_reordering(dec_struct_t *ps_dec, UWORD8 uc_lx) */ WORD32 ih264d_read_mmco_commands(struct _DecStruct * ps_dec) { + dec_pic_params_t *ps_pps = ps_dec->ps_cur_pps; + dec_seq_params_t *ps_sps = ps_pps->ps_sps; dec_bit_stream_t *ps_bitstrm = ps_dec->ps_bitstrm; dpb_commands_t *ps_dpb_cmds = &(ps_dec->s_dpb_cmds_scratch); dec_slice_params_t * ps_slice = ps_dec->ps_cur_slice; @@ -890,7 +892,7 @@ WORD32 ih264d_read_mmco_commands(struct _DecStruct * ps_dec) { UWORD32 u4_mmco; UWORD32 u4_diff_pic_num; - UWORD32 u4_lt_idx, u4_max_lt_idx; + UWORD32 u4_lt_idx, u4_max_lt_idx_plus1; u4_mmco = ih264d_uev(pu4_bitstrm_ofst, pu4_bitstrm_buf); @@ -933,9 +935,14 @@ WORD32 ih264d_read_mmco_commands(struct _DecStruct * ps_dec) case SET_MAX_LT_INDEX: { - u4_max_lt_idx = ih264d_uev(pu4_bitstrm_ofst, - pu4_bitstrm_buf); - ps_mmc_params->u4_max_lt_idx_plus1 = u4_max_lt_idx; + u4_max_lt_idx_plus1 = ih264d_uev(pu4_bitstrm_ofst, + pu4_bitstrm_buf); + if (u4_max_lt_idx_plus1 > ps_sps->u1_num_ref_frames) + { + /* Invalid max LT ref index */ + return -1; + } + ps_mmc_params->u4_max_lt_idx_plus1 = u4_max_lt_idx_plus1; break; } case RESET_REF_PICTURES: @@ -959,7 +966,6 @@ WORD32 ih264d_read_mmco_commands(struct _DecStruct * ps_dec) j++; } ps_dpb_cmds->u1_num_of_commands = j; - } } ps_dpb_cmds->u1_dpb_commands_read = 1; @@ -1243,6 +1249,13 @@ WORD32 ih264d_do_mmco_buffer(dpb_commands_t *ps_dpb_cmds, } u4_lt_idx = ps_mmc_params->u4_lt_idx; //Get long term index + + if((ps_dpb_mgr->u1_max_lt_frame_idx == NO_LONG_TERM_INDICIES) || + (u4_lt_idx > ps_dpb_mgr->u1_max_lt_frame_idx)) + { + return ERROR_DBP_MANAGER_T; + } + if(ps_dpb_mgr->u1_num_st_ref_bufs > 0) { ret = ih264d_delete_st_node_or_make_lt(ps_dpb_mgr, @@ -1257,7 +1270,7 @@ WORD32 ih264d_do_mmco_buffer(dpb_commands_t *ps_dpb_cmds, { UWORD8 uc_numLT = ps_dpb_mgr->u1_num_lt_ref_bufs; u4_lt_idx = ps_mmc_params->u4_max_lt_idx_plus1; //Get Max_long_term_index_plus1 - if(u4_lt_idx < ps_dpb_mgr->u1_max_lt_pic_idx_plus1 + if(u4_lt_idx <= ps_dpb_mgr->u1_max_lt_frame_idx && uc_numLT > 0) { struct dpb_info_t *ps_nxtDPB; @@ -1302,13 +1315,25 @@ WORD32 ih264d_do_mmco_buffer(dpb_commands_t *ps_dpb_cmds, ps_nxtDPB->ps_prev_long = NULL; } } - ps_dpb_mgr->u1_max_lt_pic_idx_plus1 = u4_lt_idx; + if(u4_lt_idx == 0) + { + ps_dpb_mgr->u1_max_lt_frame_idx = NO_LONG_TERM_INDICIES; + } + else + { + ps_dpb_mgr->u1_max_lt_frame_idx = u4_lt_idx - 1; + } break; } case SET_LT_INDEX: { u4_lt_idx = ps_mmc_params->u4_lt_idx; //Get long term index + if((ps_dpb_mgr->u1_max_lt_frame_idx == NO_LONG_TERM_INDICIES) || + (u4_lt_idx > ps_dpb_mgr->u1_max_lt_frame_idx)) + { + return ERROR_DBP_MANAGER_T; + } ret = ih264d_insert_st_node(ps_dpb_mgr, ps_pic_buf, u1_buf_id, u4_cur_pic_num); if(ret != OK) diff --git a/decoder/ih264d_utils.c b/decoder/ih264d_utils.c index 35cd7b9..1fe9a5b 100644 --- a/decoder/ih264d_utils.c +++ b/decoder/ih264d_utils.c @@ -509,6 +509,7 @@ WORD32 ih264d_end_of_pic_processing(dec_struct_t *ps_dec) ps_dec->ps_cur_pic, ps_dec->u1_pic_buf_id, ps_cur_slice->u2_frame_num); + ps_dec->ps_dpb_mgr->u1_max_lt_frame_idx = NO_LONG_TERM_INDICIES; } else { @@ -527,7 +528,7 @@ WORD32 ih264d_end_of_pic_processing(dec_struct_t *ps_dec) ps_cur_slice->u2_frame_num, 0, ps_cur_slice->u1_field_pic_flag); - ps_dec->ps_dpb_mgr->u1_max_lt_pic_idx_plus1 = 1; + ps_dec->ps_dpb_mgr->u1_max_lt_frame_idx = 0; } } } From 793a1f374f1a9ed49b52227987e53172964946a7 Mon Sep 17 00:00:00 2001 From: Shivaansh Agrawal Date: Wed, 22 Jul 2020 13:11:55 +0530 Subject: [PATCH 06/17] decoder: fix integer overflow when setting i4_prev_max_display_seq reset ps_dec->i4_prev_max_display_seq if out of int32 range to avoid overflow Bug: 143791121 Bug: 143791161 Bug: 170737173 Test: POC in bug description Merged-In: I3d8df556b003a7c739716bb33262ab3a6ca7b2d9 Change-Id: I3d8df556b003a7c739716bb33262ab3a6ca7b2d9 --- decoder/ih264d_parse_slice.c | 12 ++++++------ decoder/ih264d_utils.c | 20 ++++++++++---------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/decoder/ih264d_parse_slice.c b/decoder/ih264d_parse_slice.c index 927f1c0..d807f11 100644 --- a/decoder/ih264d_parse_slice.c +++ b/decoder/ih264d_parse_slice.c @@ -826,8 +826,8 @@ WORD32 ih264d_end_of_pic_dispbuf_mgr(dec_struct_t * ps_dec) ps_cur_pic->u2_crop_offset_uv = ps_dec->u2_crop_offset_uv; ps_cur_pic->u1_pic_type = 0; { - UWORD64 i8_display_poc; - i8_display_poc = (UWORD64)ps_dec->i4_prev_max_display_seq + + WORD64 i8_display_poc; + i8_display_poc = (WORD64)ps_dec->i4_prev_max_display_seq + ps_dec->ps_cur_pic->i4_poc; if(IS_OUT_OF_RANGE_S32(i8_display_poc)) { @@ -1495,13 +1495,13 @@ WORD32 ih264d_parse_decode_slice(UWORD8 u1_is_idr_slice, /* IDR Picture or POC wrap around */ if(i4_poc == 0) { - UWORD64 u8_temp; - u8_temp = (UWORD64)ps_dec->i4_prev_max_display_seq + WORD64 i8_temp; + i8_temp = (WORD64)ps_dec->i4_prev_max_display_seq + ps_dec->i4_max_poc + ps_dec->u1_max_dec_frame_buffering + 1; /*If i4_prev_max_display_seq overflows integer range, reset it */ - ps_dec->i4_prev_max_display_seq = (u8_temp > 0x7fffffff)? - 0 : u8_temp; + ps_dec->i4_prev_max_display_seq = IS_OUT_OF_RANGE_S32(i8_temp)? + 0 : i8_temp; ps_dec->i4_max_poc = 0; } } diff --git a/decoder/ih264d_utils.c b/decoder/ih264d_utils.c index b3f4593..0893c3f 100644 --- a/decoder/ih264d_utils.c +++ b/decoder/ih264d_utils.c @@ -1300,7 +1300,7 @@ void ih264d_release_display_bufs(dec_struct_t *ps_dec) WORD32 i4_min_poc; WORD32 i4_min_poc_buf_id; WORD32 i4_min_index; - UWORD64 u8_temp; + WORD64 i8_temp; dpb_manager_t *ps_dpb_mgr = ps_dec->ps_dpb_mgr; WORD32 (*i4_poc_buf_id_map)[3] = ps_dpb_mgr->ai4_poc_buf_id_map; @@ -1347,11 +1347,11 @@ void ih264d_release_display_bufs(dec_struct_t *ps_dec) } } ps_dpb_mgr->i1_poc_buf_id_entries = 0; - u8_temp = (UWORD64)ps_dec->i4_prev_max_display_seq + ps_dec->i4_max_poc + i8_temp = (WORD64)ps_dec->i4_prev_max_display_seq + ps_dec->i4_max_poc + ps_dec->u1_max_dec_frame_buffering + 1; /*If i4_prev_max_display_seq overflows integer range, reset it */ - ps_dec->i4_prev_max_display_seq = (u8_temp > 0x7fffffff)? - 0 : u8_temp; + ps_dec->i4_prev_max_display_seq = IS_OUT_OF_RANGE_S32(i8_temp)? + 0 : i8_temp; ps_dec->i4_max_poc = 0; } @@ -1623,13 +1623,13 @@ WORD32 ih264d_decode_gaps_in_frame_num(dec_struct_t *ps_dec, /* IDR Picture or POC wrap around */ if(i4_poc == 0) { - UWORD64 u8_temp; - u8_temp = (UWORD64)ps_dec->i4_prev_max_display_seq + WORD64 i8_temp; + i8_temp = (WORD64)ps_dec->i4_prev_max_display_seq + ps_dec->i4_max_poc + ps_dec->u1_max_dec_frame_buffering + 1; /*If i4_prev_max_display_seq overflows integer range, reset it */ - ps_dec->i4_prev_max_display_seq = (u8_temp > 0x7fffffff)? - 0 : u8_temp; + ps_dec->i4_prev_max_display_seq = IS_OUT_OF_RANGE_S32(i8_temp)? + 0 : i8_temp; ps_dec->i4_max_poc = 0; } @@ -1647,8 +1647,8 @@ WORD32 ih264d_decode_gaps_in_frame_num(dec_struct_t *ps_dec, } { - UWORD64 i8_display_poc; - i8_display_poc = (UWORD64)ps_dec->i4_prev_max_display_seq + + WORD64 i8_display_poc; + i8_display_poc = (WORD64)ps_dec->i4_prev_max_display_seq + i4_poc; if(IS_OUT_OF_RANGE_S32(i8_display_poc)) { From 06c30b33c400afcf175916dea34f8b09599b58e5 Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Tue, 17 Nov 2020 06:44:57 +0530 Subject: [PATCH 07/17] Validate input dimensions in process() Input dimensions are checked for supported range in set_dimensions control call. Encoder returns an error for unsupported values in this control call. But if the caller ignores this error and proceeds to call process(), encoder wasn't checking the dimensions again. Unsupported dimensions are now checked in process() call as well. Bug: 172908358 Test: Poc in bug Test: atest android.mediav2.cts Test: atest android.media.cts Change-Id: I677049afdd0fc4a7d7975c024d0a5a8d7758dc91 --- encoder/ih264e_api.c | 110 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/encoder/ih264e_api.c b/encoder/ih264e_api.c index 61ef6b5..53067e0 100644 --- a/encoder/ih264e_api.c +++ b/encoder/ih264e_api.c @@ -138,6 +138,101 @@ WORD32 ih264e_get_rate_control_mem_tab(void *pv_rate_control, /* Function Definitions */ /*****************************************************************************/ +/** +******************************************************************************* +* +* @brief +* Used to test validity of input dimensions +* +* @par Description: +* Dimensions of the input buffer passed to encode call are validated +* +* @param[in] ps_codec +* Codec context +* +* @param[in] ps_ip +* Pointer to input structure +* +* @param[out] ps_op +* Pointer to output structure +* +* @returns error status +* +* @remarks none +* +******************************************************************************* +*/ +static IV_STATUS_T api_check_input_dimensions(codec_t *ps_codec, + ih264e_video_encode_ip_t *ps_ip, + ih264e_video_encode_op_t *ps_op) +{ + UWORD32 u4_wd, u4_ht; + cfg_params_t *ps_curr_cfg = &ps_codec->s_cfg; + iv_raw_buf_t *ps_inp_buf = &ps_ip->s_ive_ip.s_inp_buf; + + u4_wd = ps_inp_buf->au4_wd[0]; + u4_ht = ps_inp_buf->au4_ht[0]; + switch (ps_inp_buf->e_color_fmt) + { + case IV_YUV_420P: + if (((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[1]) || + ((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[2]) || + (ps_inp_buf->au4_wd[1] != ps_inp_buf->au4_wd[2])) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; + return (IV_FAIL); + } + if (((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[1]) || + ((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[2]) || + (ps_inp_buf->au4_ht[1] != ps_inp_buf->au4_ht[2])) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; + return (IV_FAIL); + } + break; + case IV_YUV_420SP_UV: + case IV_YUV_420SP_VU: + if ((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[1]) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; + return (IV_FAIL); + } + if ((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[1]) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; + return (IV_FAIL); + } + break; + case IV_YUV_422ILE: + u4_wd = ps_inp_buf->au4_wd[0] / 2; + break; + default: + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_INPUT_CHROMA_FORMAT_NOT_SUPPORTED; + return (IV_FAIL); + } + + if (u4_wd != ps_curr_cfg->u4_disp_wd) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; + return (IV_FAIL); + } + + if (u4_ht != ps_curr_cfg->u4_disp_ht) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; + return (IV_FAIL); + } + + return IV_SUCCESS; +} + /** ******************************************************************************* * @@ -818,6 +913,7 @@ static IV_STATUS_T api_check_struct_sanity(iv_obj_t *ps_handle, case IVE_CMD_VIDEO_ENCODE: { + codec_t *ps_codec = (codec_t *) (ps_handle->pv_codec_handle); ih264e_video_encode_ip_t *ps_ip = pv_api_ip; ih264e_video_encode_op_t *ps_op = pv_api_op; @@ -836,6 +932,15 @@ static IV_STATUS_T api_check_struct_sanity(iv_obj_t *ps_handle, IVE_ERR_OP_ENCODE_API_STRUCT_SIZE_INCORRECT; return (IV_FAIL); } + + if (NULL != ps_ip->s_ive_ip.s_inp_buf.apv_bufs[0] && + ps_codec->i4_header_mode != 1 && + IV_SUCCESS != api_check_input_dimensions(ps_codec, ps_ip, ps_op)) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IVE_ERR_OP_ENCODE_API_STRUCT_SIZE_INCORRECT; + return (IV_FAIL); + } break; } @@ -4059,6 +4164,11 @@ static WORD32 ih264e_init_mem_rec(iv_obj_t *ps_codec_obj, /* Update config params as per input */ ps_cfg->u4_max_wd = ALIGN16(ps_ip->s_ive_ip.u4_max_wd); ps_cfg->u4_max_ht = ALIGN16(ps_ip->s_ive_ip.u4_max_ht); + + /* Initialize dimensions to max dimensions during init */ + ps_cfg->u4_wd = ps_cfg->u4_disp_wd = ps_cfg->u4_max_wd; + ps_cfg->u4_ht = ps_cfg->u4_disp_ht = ps_cfg->u4_max_ht; + ps_cfg->i4_wd_mbs = ps_cfg->u4_max_wd >> 4; ps_cfg->i4_ht_mbs = ps_cfg->u4_max_ht >> 4; ps_cfg->u4_max_ref_cnt = ps_ip->s_ive_ip.u4_max_ref_cnt; From 5429105538e10f40547b1265b395a01847a84720 Mon Sep 17 00:00:00 2001 From: Ray Essick Date: Mon, 21 Dec 2020 17:24:42 +0000 Subject: [PATCH 08/17] Revert "Validate input dimensions in process()" This reverts commit 06c30b33c400afcf175916dea34f8b09599b58e5. Reason for revert: buildcop flagged test failure in mediatranscoding Bug: 176041187 Bug: 172908358 Change-Id: If813107ea7a2c9f3a87886504c7e36b4473c37db --- encoder/ih264e_api.c | 110 ------------------------------------------- 1 file changed, 110 deletions(-) diff --git a/encoder/ih264e_api.c b/encoder/ih264e_api.c index 53067e0..61ef6b5 100644 --- a/encoder/ih264e_api.c +++ b/encoder/ih264e_api.c @@ -138,101 +138,6 @@ WORD32 ih264e_get_rate_control_mem_tab(void *pv_rate_control, /* Function Definitions */ /*****************************************************************************/ -/** -******************************************************************************* -* -* @brief -* Used to test validity of input dimensions -* -* @par Description: -* Dimensions of the input buffer passed to encode call are validated -* -* @param[in] ps_codec -* Codec context -* -* @param[in] ps_ip -* Pointer to input structure -* -* @param[out] ps_op -* Pointer to output structure -* -* @returns error status -* -* @remarks none -* -******************************************************************************* -*/ -static IV_STATUS_T api_check_input_dimensions(codec_t *ps_codec, - ih264e_video_encode_ip_t *ps_ip, - ih264e_video_encode_op_t *ps_op) -{ - UWORD32 u4_wd, u4_ht; - cfg_params_t *ps_curr_cfg = &ps_codec->s_cfg; - iv_raw_buf_t *ps_inp_buf = &ps_ip->s_ive_ip.s_inp_buf; - - u4_wd = ps_inp_buf->au4_wd[0]; - u4_ht = ps_inp_buf->au4_ht[0]; - switch (ps_inp_buf->e_color_fmt) - { - case IV_YUV_420P: - if (((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[1]) || - ((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[2]) || - (ps_inp_buf->au4_wd[1] != ps_inp_buf->au4_wd[2])) - { - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; - return (IV_FAIL); - } - if (((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[1]) || - ((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[2]) || - (ps_inp_buf->au4_ht[1] != ps_inp_buf->au4_ht[2])) - { - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; - return (IV_FAIL); - } - break; - case IV_YUV_420SP_UV: - case IV_YUV_420SP_VU: - if ((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[1]) - { - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; - return (IV_FAIL); - } - if ((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[1]) - { - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; - return (IV_FAIL); - } - break; - case IV_YUV_422ILE: - u4_wd = ps_inp_buf->au4_wd[0] / 2; - break; - default: - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IH264E_INPUT_CHROMA_FORMAT_NOT_SUPPORTED; - return (IV_FAIL); - } - - if (u4_wd != ps_curr_cfg->u4_disp_wd) - { - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; - return (IV_FAIL); - } - - if (u4_ht != ps_curr_cfg->u4_disp_ht) - { - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; - return (IV_FAIL); - } - - return IV_SUCCESS; -} - /** ******************************************************************************* * @@ -913,7 +818,6 @@ static IV_STATUS_T api_check_struct_sanity(iv_obj_t *ps_handle, case IVE_CMD_VIDEO_ENCODE: { - codec_t *ps_codec = (codec_t *) (ps_handle->pv_codec_handle); ih264e_video_encode_ip_t *ps_ip = pv_api_ip; ih264e_video_encode_op_t *ps_op = pv_api_op; @@ -932,15 +836,6 @@ static IV_STATUS_T api_check_struct_sanity(iv_obj_t *ps_handle, IVE_ERR_OP_ENCODE_API_STRUCT_SIZE_INCORRECT; return (IV_FAIL); } - - if (NULL != ps_ip->s_ive_ip.s_inp_buf.apv_bufs[0] && - ps_codec->i4_header_mode != 1 && - IV_SUCCESS != api_check_input_dimensions(ps_codec, ps_ip, ps_op)) - { - ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; - ps_op->s_ive_op.u4_error_code |= IVE_ERR_OP_ENCODE_API_STRUCT_SIZE_INCORRECT; - return (IV_FAIL); - } break; } @@ -4164,11 +4059,6 @@ static WORD32 ih264e_init_mem_rec(iv_obj_t *ps_codec_obj, /* Update config params as per input */ ps_cfg->u4_max_wd = ALIGN16(ps_ip->s_ive_ip.u4_max_wd); ps_cfg->u4_max_ht = ALIGN16(ps_ip->s_ive_ip.u4_max_ht); - - /* Initialize dimensions to max dimensions during init */ - ps_cfg->u4_wd = ps_cfg->u4_disp_wd = ps_cfg->u4_max_wd; - ps_cfg->u4_ht = ps_cfg->u4_disp_ht = ps_cfg->u4_max_ht; - ps_cfg->i4_wd_mbs = ps_cfg->u4_max_wd >> 4; ps_cfg->i4_ht_mbs = ps_cfg->u4_max_ht >> 4; ps_cfg->u4_max_ref_cnt = ps_ip->s_ive_ip.u4_max_ref_cnt; From 3de25bf0bbd2039421e191ff936a31f3900460ae Mon Sep 17 00:00:00 2001 From: Neelkamal Semwal Date: Wed, 10 Mar 2021 10:03:39 +0530 Subject: [PATCH 09/17] avcenc: Add bitstream overflow check during emulation prevention Bug: 176533109 Test: poc in the bug description Change-Id: Ia83383f9b65cbde8d7a50a1af8a054936daa4d78 (cherry picked from commit b59de5a25f28f0fe411526b2e50bb8052957c517) --- encoder/ih264e_bitstream.c | 51 ++++----------------------- encoder/ih264e_bitstream.h | 72 ++++++++++++++++++++++++++------------ encoder/ih264e_cabac.c | 61 ++++++++++++++------------------ encoder/ih264e_cabac.h | 4 +-- encoder/ih264e_process.c | 4 +-- 5 files changed, 86 insertions(+), 106 deletions(-) diff --git a/encoder/ih264e_bitstream.c b/encoder/ih264e_bitstream.c index d79f637..9f73f69 100644 --- a/encoder/ih264e_bitstream.c +++ b/encoder/ih264e_bitstream.c @@ -182,27 +182,9 @@ IH264E_ERROR_T ih264e_put_bits(bitstrm_t *ps_bitstrm, /* 4. insert remaining bits of code starting from msb of cur word */ /* 5. update bitsleft in current word and stream buffer offset */ /********************************************************************/ - UWORD32 u4_strm_buf_offset = ps_bitstrm->u4_strm_buf_offset; - - UWORD32 u4_max_strm_size = ps_bitstrm->u4_max_strm_size; - - WORD32 zero_run = ps_bitstrm->i4_zero_bytes_run; - - UWORD8* pu1_strm_buf = ps_bitstrm->pu1_strm_buffer; - + IH264E_ERROR_T status = IH264E_SUCCESS; WORD32 i, rem_bits = (code_len - bits_left_in_cw); - - /*********************************************************************/ - /* Bitstream overflow check */ - /* NOTE: corner case of epb bytes (max 2 for 32bit word) not handled */ - /*********************************************************************/ - if((u4_strm_buf_offset + (WORD_SIZE>>3)) >= u4_max_strm_size) - { - /* return without corrupting the buffer beyond its size */ - return(IH264E_BITSTREAM_BUFFER_OVERFLOW); - } - /* insert parital code corresponding to bits left in cur word */ u4_cur_word |= u4_code_val >> rem_bits; @@ -211,7 +193,7 @@ IH264E_ERROR_T ih264e_put_bits(bitstrm_t *ps_bitstrm, /* flush the bits in cur word byte by byte and copy to stream */ UWORD8 u1_next_byte = (u4_cur_word >> (i-8)) & 0xFF; - PUTBYTE_EPB(pu1_strm_buf, u4_strm_buf_offset, u1_next_byte, zero_run); + status |= ih264e_put_byte_epb(ps_bitstrm, u1_next_byte); } /* insert the remaining bits from code val into current word */ @@ -220,9 +202,8 @@ IH264E_ERROR_T ih264e_put_bits(bitstrm_t *ps_bitstrm, /* update the state variables and return success */ ps_bitstrm->u4_cur_word = u4_cur_word; ps_bitstrm->i4_bits_left_in_cw = WORD_SIZE - rem_bits; - ps_bitstrm->i4_zero_bytes_run = zero_run; - ps_bitstrm->u4_strm_buf_offset = u4_strm_buf_offset; - return (IH264E_SUCCESS); + return (status); + } } @@ -281,22 +262,7 @@ IH264E_ERROR_T ih264e_put_rbsp_trailing_bits(bitstrm_t *ps_bitstrm) UWORD32 u4_cur_word = ps_bitstrm->u4_cur_word; WORD32 bits_left_in_cw = ps_bitstrm->i4_bits_left_in_cw; WORD32 bytes_left_in_cw = (bits_left_in_cw - 1) >> 3; - - UWORD32 u4_strm_buf_offset = ps_bitstrm->u4_strm_buf_offset; - UWORD32 u4_max_strm_size = ps_bitstrm->u4_max_strm_size; - WORD32 zero_run = ps_bitstrm->i4_zero_bytes_run; - UWORD8* pu1_strm_buf = ps_bitstrm->pu1_strm_buffer; - - /*********************************************************************/ - /* Bitstream overflow check */ - /* NOTE: corner case of epb bytes (max 2 for 32bit word) not handled */ - /*********************************************************************/ - if((u4_strm_buf_offset + (WORD_SIZE>>3) - bytes_left_in_cw) >= - u4_max_strm_size) - { - /* return without corrupting the buffer beyond its size */ - return(IH264E_BITSTREAM_BUFFER_OVERFLOW); - } + IH264E_ERROR_T status = IH264E_SUCCESS; /* insert a 1 at the end of current word and flush all the bits */ u4_cur_word |= (1 << (bits_left_in_cw - 1)); @@ -309,18 +275,15 @@ IH264E_ERROR_T ih264e_put_rbsp_trailing_bits(bitstrm_t *ps_bitstrm) /* flush the bits in cur word byte by byte and copy to stream */ UWORD8 u1_next_byte = (u4_cur_word >> (i-8)) & 0xFF; - PUTBYTE_EPB(pu1_strm_buf, u4_strm_buf_offset, u1_next_byte, zero_run); + status |= ih264e_put_byte_epb(ps_bitstrm, u1_next_byte); } - /* update the stream offset */ - ps_bitstrm->u4_strm_buf_offset = u4_strm_buf_offset; - /* Default init values for scratch variables of bitstream context */ ps_bitstrm->u4_cur_word = 0; ps_bitstrm->i4_bits_left_in_cw = WORD_SIZE; ps_bitstrm->i4_zero_bytes_run = 0; - return (IH264E_SUCCESS); + return (status); } /** diff --git a/encoder/ih264e_bitstream.h b/encoder/ih264e_bitstream.h index 9cd2b81..5b5c700 100644 --- a/encoder/ih264e_bitstream.h +++ b/encoder/ih264e_bitstream.h @@ -169,29 +169,7 @@ { \ const WORD32 len = (WORD32)((ps_bitstrm->i4_bits_left_in_cw) & 0x07);\ ih264e_put_bits(ps_bitstrm, (UWORD32)((1 << len) - 1), len); \ - } - - -/** -****************************************************************************** -* flush the bits in cur word byte by byte and copy to stream * -* (current word is assumed to be byte aligned) * -****************************************************************************** -*/ -#define BITSTREAM_FLUSH(ps_bitstrm) \ -{ \ - WORD32 i; \ - for (i = WORD_SIZE; i > ps_bitstrm->i4_bits_left_in_cw; i -= 8) \ - { \ - UWORD8 u1_next_byte = (ps_bitstrm->u4_cur_word >> (i - 8)) & 0xFF; \ - PUTBYTE_EPB(ps_bitstrm->pu1_strm_buffer, ps_bitstrm->u4_strm_buf_offset,\ - u1_next_byte, ps_bitstrm->i4_zero_bytes_run); \ - } \ - ps_bitstrm->u4_cur_word = 0; \ - ps_bitstrm->i4_bits_left_in_cw = WORD_SIZE; \ -} \ - - + } \ /*****************************************************************************/ @@ -245,6 +223,54 @@ typedef struct bitstrm } bitstrm_t; +/** +****************************************************************************** +* @brief Inserts 1 byte and Emulation Prevention Byte(if any) into bitstream +* Increments the stream offset and zero run correspondingly +****************************************************************************** +*/ +static inline IH264E_ERROR_T ih264e_put_byte_epb(bitstrm_t *ps_bitstrm, UWORD8 byte) +{ + if (INSERT_EPB(ps_bitstrm->i4_zero_bytes_run, byte)) + { + if ((ps_bitstrm->u4_strm_buf_offset + 1) >= ps_bitstrm->u4_max_strm_size) + { + return IH264E_BITSTREAM_BUFFER_OVERFLOW; + } + ps_bitstrm->pu1_strm_buffer[ps_bitstrm->u4_strm_buf_offset++] = EPB_BYTE; + ps_bitstrm->i4_zero_bytes_run = 0; + } + + if ((ps_bitstrm->u4_strm_buf_offset + 1) >= ps_bitstrm->u4_max_strm_size) + { + return IH264E_BITSTREAM_BUFFER_OVERFLOW; + } + ps_bitstrm->pu1_strm_buffer[ps_bitstrm->u4_strm_buf_offset++] = byte; + ps_bitstrm->i4_zero_bytes_run = byte ? 0 : ps_bitstrm->i4_zero_bytes_run + 1; + + return IH264E_SUCCESS; +} + +/** +****************************************************************************** +* flush the bits in cur word byte by byte and copy to stream * +* (current word is assumed to be byte aligned) * +****************************************************************************** +*/ +#define BITSTREAM_FLUSH(ps_bitstrm, err) \ +{ \ + WORD32 i; \ + for (i = WORD_SIZE; i > ps_bitstrm->i4_bits_left_in_cw; i -= 8) \ + { \ + UWORD8 u1_next_byte = (ps_bitstrm->u4_cur_word >> (i - 8)) & 0xFF; \ + err |= ih264e_put_byte_epb(ps_bitstrm, u1_next_byte); \ + } \ + ps_bitstrm->u4_cur_word = 0; \ + ps_bitstrm->i4_bits_left_in_cw = WORD_SIZE; \ +} \ + + + /*****************************************************************************/ /* Extern Function Declarations */ /*****************************************************************************/ diff --git a/encoder/ih264e_cabac.c b/encoder/ih264e_cabac.c index 26ded4d..2d91058 100644 --- a/encoder/ih264e_cabac.c +++ b/encoder/ih264e_cabac.c @@ -242,18 +242,16 @@ void ih264e_get_cabac_context(entropy_ctxt_t *ps_ent_ctxt, WORD32 u4_mb_type) * ******************************************************************************* */ -void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt) +IH264E_ERROR_T ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt) { - /* bit stream ptr */ bitstrm_t *ps_stream = ps_cabac_ctxt->ps_bitstrm; encoding_envirnoment_t *ps_cab_enc_env = &(ps_cabac_ctxt->s_cab_enc_env); UWORD32 u4_low = ps_cab_enc_env->u4_code_int_low; UWORD32 u4_bits_gen = ps_cab_enc_env->u4_bits_gen; UWORD8 *pu1_strm_buf = ps_stream->pu1_strm_buffer; - UWORD32 u4_strm_buf_offset = ps_stream->u4_strm_buf_offset; - WORD32 zero_run = ps_stream->i4_zero_bytes_run; UWORD32 u4_out_standing_bytes = ps_cab_enc_env->u4_out_standing_bytes; + IH264E_ERROR_T status = IH264E_SUCCESS; /************************************************************************/ /* Insert the carry (propogated in previous byte) along with */ @@ -274,17 +272,17 @@ void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt) as per standard */ /* so check for previous four bytes and if it is equal to 0x00000303 then subtract u4_strm_buf_offset by 1 */ - if (pu1_strm_buf[u4_strm_buf_offset - 1] == 0x03 - && pu1_strm_buf[u4_strm_buf_offset - 2] == 0x03 - && pu1_strm_buf[u4_strm_buf_offset - 3] == 0x00 - && pu1_strm_buf[u4_strm_buf_offset - 4] == 0x00) + if (pu1_strm_buf[ps_stream->u4_strm_buf_offset - 1] == 0x03 + && pu1_strm_buf[ps_stream->u4_strm_buf_offset - 2] == 0x03 + && pu1_strm_buf[ps_stream->u4_strm_buf_offset - 3] == 0x00 + && pu1_strm_buf[ps_stream->u4_strm_buf_offset - 4] == 0x00) { - u4_strm_buf_offset -= 1; + ps_stream->u4_strm_buf_offset -= 1; } /* previous byte carry add will not result in overflow to */ /* u4_strm_buf_offset - 2 as we track 0xff as outstanding bytes */ - pu1_strm_buf[u4_strm_buf_offset - 1] += carry; - zero_run = 0; + pu1_strm_buf[ps_stream->u4_strm_buf_offset - 1] += carry; + ps_stream->i4_zero_bytes_run = 0; } /* Insert outstanding bytes (if any) */ @@ -292,7 +290,7 @@ void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt) { UWORD8 u1_0_or_ff = carry ? 0 : 0xFF; - PUTBYTE_EPB(pu1_strm_buf, u4_strm_buf_offset, u1_0_or_ff, zero_run); + status |= ih264e_put_byte_epb(ps_stream, u1_0_or_ff); u4_out_standing_bytes--; } @@ -307,7 +305,7 @@ void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt) if (bits_left >= 8) { last_byte = (rem_bits >> (bits_left - 8)) & 0xFF; - PUTBYTE_EPB(pu1_strm_buf, u4_strm_buf_offset, last_byte, zero_run); + status |= ih264e_put_byte_epb(ps_stream, last_byte); bits_left -= 8; } @@ -315,16 +313,16 @@ void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt) last_byte = (rem_bits << (8 - bits_left)) | (1 << (7 - bits_left) | (1 << (7 - bits_left - 1))); last_byte &= 0xFF; - PUTBYTE_EPB(pu1_strm_buf, u4_strm_buf_offset, last_byte, zero_run); + status |= ih264e_put_byte_epb(ps_stream, last_byte); /* update the state variables and return success */ - ps_stream->u4_strm_buf_offset = u4_strm_buf_offset; ps_stream->i4_zero_bytes_run = 0; /* Default init values for scratch variables of bitstream context */ ps_stream->u4_cur_word = 0; ps_stream->i4_bits_left_in_cw = WORD_SIZE; } + return status; } /** @@ -349,15 +347,16 @@ void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt) * ****************************************************************************** */ -void ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt) +IH264E_ERROR_T ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt) { - /* bit stream ptr */ bitstrm_t *ps_stream = ps_cabac_ctxt->ps_bitstrm; encoding_envirnoment_t *ps_cab_enc_env = &(ps_cabac_ctxt->s_cab_enc_env); UWORD32 u4_low = ps_cab_enc_env->u4_code_int_low; UWORD32 u4_bits_gen = ps_cab_enc_env->u4_bits_gen; + UWORD8 *pu1_strm_buf = ps_stream->pu1_strm_buffer; WORD32 lead_byte = u4_low >> (u4_bits_gen + CABAC_BITS - 8); + IH264E_ERROR_T status = IH264E_SUCCESS; /* Sanity checks */ ASSERT((ps_cab_enc_env->u4_code_int_range >= 256) @@ -381,15 +380,11 @@ void ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt) { /* actual bits depend on carry propogration */ ps_cab_enc_env->u4_out_standing_bytes++; - return ; } else { /* carry = 1 => putbit(1); carry propogated due to L renorm */ WORD32 carry = (lead_byte >> 8) & 0x1; - UWORD8 *pu1_strm_buf = ps_stream->pu1_strm_buffer; - UWORD32 u4_strm_buf_offset = ps_stream->u4_strm_buf_offset; - WORD32 zero_run = ps_stream->i4_zero_bytes_run; UWORD32 u4_out_standing_bytes = ps_cab_enc_env->u4_out_standing_bytes; @@ -407,17 +402,17 @@ void ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt) as per standard */ /* so check for previous four bytes and if it is equal to 0x00000303 then subtract u4_strm_buf_offset by 1 */ - if (pu1_strm_buf[u4_strm_buf_offset - 1] == 0x03 - && pu1_strm_buf[u4_strm_buf_offset - 2] == 0x03 - && pu1_strm_buf[u4_strm_buf_offset - 3] == 0x00 - && pu1_strm_buf[u4_strm_buf_offset - 4] == 0x00) + if (pu1_strm_buf[ps_stream->u4_strm_buf_offset - 1] == 0x03 + && pu1_strm_buf[ps_stream->u4_strm_buf_offset - 2] == 0x03 + && pu1_strm_buf[ps_stream->u4_strm_buf_offset - 3] == 0x00 + && pu1_strm_buf[ps_stream->u4_strm_buf_offset - 4] == 0x00) { - u4_strm_buf_offset -= 1; + ps_stream->u4_strm_buf_offset -= 1; } /* previous byte carry add will not result in overflow to */ /* u4_strm_buf_offset - 2 as we track 0xff as outstanding bytes */ - pu1_strm_buf[u4_strm_buf_offset - 1] += carry; - zero_run = 0; + pu1_strm_buf[ps_stream->u4_strm_buf_offset - 1] += carry; + ps_stream->i4_zero_bytes_run = 0; } /* Insert outstanding bytes (if any) */ @@ -425,7 +420,7 @@ void ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt) { UWORD8 u1_0_or_ff = carry ? 0 : 0xFF; - PUTBYTE_EPB(pu1_strm_buf, u4_strm_buf_offset, u1_0_or_ff, zero_run); + status |= ih264e_put_byte_epb(ps_stream, u1_0_or_ff); u4_out_standing_bytes--; } @@ -433,13 +428,9 @@ void ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt) /* Insert the leading byte */ lead_byte &= 0xFF; - PUTBYTE_EPB(pu1_strm_buf, u4_strm_buf_offset, lead_byte, zero_run); - - /* update the state variables and return success */ - ps_stream->u4_strm_buf_offset = u4_strm_buf_offset; - ps_stream->i4_zero_bytes_run = zero_run; - + status |= ih264e_put_byte_epb(ps_stream, lead_byte); } + return status; } diff --git a/encoder/ih264e_cabac.h b/encoder/ih264e_cabac.h index e4722fa..bc4b07c 100644 --- a/encoder/ih264e_cabac.h +++ b/encoder/ih264e_cabac.h @@ -190,7 +190,7 @@ void ih264e_get_cabac_context(entropy_ctxt_t *ps_ent_ctxt, WORD32 u4_mb_type); * ******************************************************************************* */ -void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt); +IH264E_ERROR_T ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt); /** @@ -215,7 +215,7 @@ void ih264e_cabac_flush(cabac_ctxt_t *ps_cabac_ctxt); * ****************************************************************************** */ -void ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt); +IH264E_ERROR_T ih264e_cabac_put_byte(cabac_ctxt_t *ps_cabac_ctxt); /** diff --git a/encoder/ih264e_process.c b/encoder/ih264e_process.c index 490c6d8..289053f 100644 --- a/encoder/ih264e_process.c +++ b/encoder/ih264e_process.c @@ -444,7 +444,7 @@ IH264E_ERROR_T ih264e_entropy(process_ctxt_t *ps_proc) if (CABAC == ps_entropy->u1_entropy_coding_mode_flag) { BITSTREAM_BYTE_ALIGN(ps_bitstrm); - BITSTREAM_FLUSH(ps_bitstrm); + BITSTREAM_FLUSH(ps_bitstrm, ps_entropy->i4_error_code); ih264e_init_cabac_ctxt(ps_entropy); } } @@ -571,7 +571,7 @@ IH264E_ERROR_T ih264e_entropy(process_ctxt_t *ps_proc) if (CABAC == ps_entropy->u1_entropy_coding_mode_flag) { BITSTREAM_BYTE_ALIGN(ps_bitstrm); - BITSTREAM_FLUSH(ps_bitstrm); + BITSTREAM_FLUSH(ps_bitstrm, ps_entropy->i4_error_code); ih264e_init_cabac_ctxt(ps_entropy); } } From daa5c36657ad4c7399b4117864639c73e6f953c5 Mon Sep 17 00:00:00 2001 From: Harish Mahendrakar Date: Tue, 17 Nov 2020 06:44:57 +0530 Subject: [PATCH 10/17] Validate input dimensions in process() Input dimensions are checked for supported range in set_dimensions control call. Encoder returns an error for unsupported values in this control call. But if the caller ignores this error and proceeds to call process(), encoder wasn't checking the dimensions again. Unsupported dimensions are now checked in process() call as well. Note: This is relanding previously reverted commit 06c30b33c400afcf175916dea34f8b09599b58e5 Bug: 172908358 Test: Poc in bug Test: atest android.mediav2.cts Test: atest android.media.cts Change-Id: Icf3f296ab24432c680427a82da3505491acca3bd --- encoder/ih264e_api.c | 110 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/encoder/ih264e_api.c b/encoder/ih264e_api.c index 61ef6b5..53067e0 100644 --- a/encoder/ih264e_api.c +++ b/encoder/ih264e_api.c @@ -138,6 +138,101 @@ WORD32 ih264e_get_rate_control_mem_tab(void *pv_rate_control, /* Function Definitions */ /*****************************************************************************/ +/** +******************************************************************************* +* +* @brief +* Used to test validity of input dimensions +* +* @par Description: +* Dimensions of the input buffer passed to encode call are validated +* +* @param[in] ps_codec +* Codec context +* +* @param[in] ps_ip +* Pointer to input structure +* +* @param[out] ps_op +* Pointer to output structure +* +* @returns error status +* +* @remarks none +* +******************************************************************************* +*/ +static IV_STATUS_T api_check_input_dimensions(codec_t *ps_codec, + ih264e_video_encode_ip_t *ps_ip, + ih264e_video_encode_op_t *ps_op) +{ + UWORD32 u4_wd, u4_ht; + cfg_params_t *ps_curr_cfg = &ps_codec->s_cfg; + iv_raw_buf_t *ps_inp_buf = &ps_ip->s_ive_ip.s_inp_buf; + + u4_wd = ps_inp_buf->au4_wd[0]; + u4_ht = ps_inp_buf->au4_ht[0]; + switch (ps_inp_buf->e_color_fmt) + { + case IV_YUV_420P: + if (((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[1]) || + ((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[2]) || + (ps_inp_buf->au4_wd[1] != ps_inp_buf->au4_wd[2])) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; + return (IV_FAIL); + } + if (((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[1]) || + ((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[2]) || + (ps_inp_buf->au4_ht[1] != ps_inp_buf->au4_ht[2])) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; + return (IV_FAIL); + } + break; + case IV_YUV_420SP_UV: + case IV_YUV_420SP_VU: + if ((ps_inp_buf->au4_wd[0] / 2) != ps_inp_buf->au4_wd[1]) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; + return (IV_FAIL); + } + if ((ps_inp_buf->au4_ht[0] / 2) != ps_inp_buf->au4_ht[1]) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; + return (IV_FAIL); + } + break; + case IV_YUV_422ILE: + u4_wd = ps_inp_buf->au4_wd[0] / 2; + break; + default: + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_INPUT_CHROMA_FORMAT_NOT_SUPPORTED; + return (IV_FAIL); + } + + if (u4_wd != ps_curr_cfg->u4_disp_wd) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_WIDTH_NOT_SUPPORTED; + return (IV_FAIL); + } + + if (u4_ht != ps_curr_cfg->u4_disp_ht) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IH264E_HEIGHT_NOT_SUPPORTED; + return (IV_FAIL); + } + + return IV_SUCCESS; +} + /** ******************************************************************************* * @@ -818,6 +913,7 @@ static IV_STATUS_T api_check_struct_sanity(iv_obj_t *ps_handle, case IVE_CMD_VIDEO_ENCODE: { + codec_t *ps_codec = (codec_t *) (ps_handle->pv_codec_handle); ih264e_video_encode_ip_t *ps_ip = pv_api_ip; ih264e_video_encode_op_t *ps_op = pv_api_op; @@ -836,6 +932,15 @@ static IV_STATUS_T api_check_struct_sanity(iv_obj_t *ps_handle, IVE_ERR_OP_ENCODE_API_STRUCT_SIZE_INCORRECT; return (IV_FAIL); } + + if (NULL != ps_ip->s_ive_ip.s_inp_buf.apv_bufs[0] && + ps_codec->i4_header_mode != 1 && + IV_SUCCESS != api_check_input_dimensions(ps_codec, ps_ip, ps_op)) + { + ps_op->s_ive_op.u4_error_code |= 1 << IVE_UNSUPPORTEDPARAM; + ps_op->s_ive_op.u4_error_code |= IVE_ERR_OP_ENCODE_API_STRUCT_SIZE_INCORRECT; + return (IV_FAIL); + } break; } @@ -4059,6 +4164,11 @@ static WORD32 ih264e_init_mem_rec(iv_obj_t *ps_codec_obj, /* Update config params as per input */ ps_cfg->u4_max_wd = ALIGN16(ps_ip->s_ive_ip.u4_max_wd); ps_cfg->u4_max_ht = ALIGN16(ps_ip->s_ive_ip.u4_max_ht); + + /* Initialize dimensions to max dimensions during init */ + ps_cfg->u4_wd = ps_cfg->u4_disp_wd = ps_cfg->u4_max_wd; + ps_cfg->u4_ht = ps_cfg->u4_disp_ht = ps_cfg->u4_max_ht; + ps_cfg->i4_wd_mbs = ps_cfg->u4_max_wd >> 4; ps_cfg->i4_ht_mbs = ps_cfg->u4_max_ht >> 4; ps_cfg->u4_max_ref_cnt = ps_ip->s_ive_ip.u4_max_ref_cnt; From e88c22c34cd93eef75ae473c4a35af775546b8fe Mon Sep 17 00:00:00 2001 From: Aayush Soni Date: Thu, 25 Mar 2021 16:59:10 +0530 Subject: [PATCH 11/17] encoder: fix null buffer dereferencing Do not set the picture type as NA for the last frame in encoding order. Bug: 180219345 Test: POC in bug descriptions Test: atest CtsMediaV2TestCases:CodecEncoderTest Test: atest VtsHalMediaC2V1_0TargetVideoEncTest Change-Id: I57ee7db9e2e55fba2666a6aaa4002ef31276e4b9 --- encoder/ih264e_utils.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/encoder/ih264e_utils.c b/encoder/ih264e_utils.c index 2196a64..239271e 100644 --- a/encoder/ih264e_utils.c +++ b/encoder/ih264e_utils.c @@ -445,11 +445,6 @@ WORD32 ih264e_input_queue_update(codec_t *ps_codec, } } - if (ps_enc_buff->u4_is_last) - { - ps_codec->pic_type = PIC_NA; - } - /* The buffer in the queue is set to NULL to specify that encoding is done for that frame */ for(i = 0; i < 3; i++) { From 954f023c74425d097c7ee45c1d9a37a7fafe778c Mon Sep 17 00:00:00 2001 From: Neelkamal Semwal Date: Fri, 12 Mar 2021 10:15:49 +0530 Subject: [PATCH 12/17] encoder: fix invalid free of raw buffers Return current input buffer as buffer to be freed in case of errors that are seen before picking up the input buffer to be from the input queue. Once a buffer is picked up from the queue, that is returned as the buffer to be freed. There is no need to return a buffer from ps_proc context Bug: 180643802 Test: poc in the bug description Test: atest CtsMediaV2TestCases:CodecEncoderTest Test: atest VtsHalMediaC2V1_0TargetVideoEncTest Change-Id: I1671ca1e82f522004d1f070df89b256b856f75b8 --- encoder/ih264e_encode.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/encoder/ih264e_encode.c b/encoder/ih264e_encode.c index 9210b3e..6a4e3a2 100644 --- a/encoder/ih264e_encode.c +++ b/encoder/ih264e_encode.c @@ -228,6 +228,9 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) ps_video_encode_op->s_ive_op.output_present = 0; ps_video_encode_op->s_ive_op.dump_recon = 0; ps_video_encode_op->s_ive_op.u4_encoded_frame_type = IV_NA_FRAME; + /* By default set the current input buffer as the buffer to be freed */ + /* This will later be updated to the actual input that gets encoded */ + ps_video_encode_op->s_ive_op.s_inp_buf = ps_video_encode_ip->s_ive_ip.s_inp_buf; /* Check for output memory allocation size */ if (ps_video_encode_ip->s_ive_ip.s_out_buf.u4_bufsize < MIN_STREAM_SIZE) @@ -474,6 +477,9 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) s_out_buf.u4_is_last = s_inp_buf.u4_is_last; ps_video_encode_op->s_ive_op.u4_is_last = s_inp_buf.u4_is_last; + /* Send the input to application so that it can free it */ + ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; + /* Only encode if the current frame is not pre-encode skip */ if (!i4_rc_pre_enc_skip && s_inp_buf.s_raw_buf.apv_bufs[0]) { @@ -774,12 +780,6 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) } else { - /* proc ctxt base idx */ - WORD32 proc_ctxt_select = ctxt_sel * MAX_PROCESS_THREADS; - - /* proc ctxt */ - process_ctxt_t *ps_proc = &ps_codec->as_process[proc_ctxt_select]; - /* receive output back from codec */ s_out_buf = ps_codec->as_out_buf[ctxt_sel]; @@ -790,18 +790,11 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) ps_video_encode_op->s_ive_op.u4_timestamp_low = 0; ps_video_encode_op->s_ive_op.u4_timestamp_high = 0; - /* receive input back from codec and send it to app */ - s_inp_buf = ps_proc->s_inp_buf; - ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; - ps_video_encode_op->s_ive_op.u4_encoded_frame_type = IV_NA_FRAME; } - /* Send the input to encoder so that it can free it if possible */ ps_video_encode_op->s_ive_op.s_out_buf = s_out_buf.s_bits_buf; - ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; - if (1 == s_inp_buf.u4_is_last) { From 3e73f0d56298ba6256927928669d0cc6e4b1c9ee Mon Sep 17 00:00:00 2001 From: Neelkamal Semwal Date: Fri, 12 Mar 2021 10:15:49 +0530 Subject: [PATCH 13/17] encoder: fix invalid free of raw buffers Return current input buffer as buffer to be freed in case of errors that are seen before picking up the input buffer to be from the input queue. Once a buffer is picked up from the queue, that is returned as the buffer to be freed. There is no need to return a buffer from ps_proc context Bug: 180643802 Test: poc in the bug description Test: atest CtsMediaV2TestCases:CodecEncoderTest Test: atest VtsHalMediaC2V1_0TargetVideoEncTest Change-Id: I1671ca1e82f522004d1f070df89b256b856f75b8 --- encoder/ih264e_encode.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/encoder/ih264e_encode.c b/encoder/ih264e_encode.c index e7057dc..c7e3717 100644 --- a/encoder/ih264e_encode.c +++ b/encoder/ih264e_encode.c @@ -225,6 +225,9 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) ps_video_encode_op->s_ive_op.output_present = 0; ps_video_encode_op->s_ive_op.dump_recon = 0; ps_video_encode_op->s_ive_op.u4_encoded_frame_type = IV_NA_FRAME; + /* By default set the current input buffer as the buffer to be freed */ + /* This will later be updated to the actual input that gets encoded */ + ps_video_encode_op->s_ive_op.s_inp_buf = ps_video_encode_ip->s_ive_ip.s_inp_buf; /* Check for output memory allocation size */ if (ps_video_encode_ip->s_ive_ip.s_out_buf.u4_bufsize < MIN_STREAM_SIZE) @@ -394,6 +397,9 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) s_out_buf.u4_is_last = s_inp_buf.u4_is_last; ps_video_encode_op->s_ive_op.u4_is_last = s_inp_buf.u4_is_last; + /* Send the input to application so that it can free it */ + ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; + /* Only encode if the current frame is not pre-encode skip */ if (!i4_rc_pre_enc_skip && s_inp_buf.s_raw_buf.apv_bufs[0]) { @@ -694,12 +700,6 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) } else { - /* proc ctxt base idx */ - WORD32 proc_ctxt_select = ctxt_sel * MAX_PROCESS_THREADS; - - /* proc ctxt */ - process_ctxt_t *ps_proc = &ps_codec->as_process[proc_ctxt_select]; - /* receive output back from codec */ s_out_buf = ps_codec->as_out_buf[ctxt_sel]; @@ -710,18 +710,11 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) ps_video_encode_op->s_ive_op.u4_timestamp_low = 0; ps_video_encode_op->s_ive_op.u4_timestamp_high = 0; - /* receive input back from codec and send it to app */ - s_inp_buf = ps_proc->s_inp_buf; - ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; - ps_video_encode_op->s_ive_op.u4_encoded_frame_type = IV_NA_FRAME; } - /* Send the input to encoder so that it can free it if possible */ ps_video_encode_op->s_ive_op.s_out_buf = s_out_buf.s_bits_buf; - ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; - if (1 == s_inp_buf.u4_is_last) { From e1fce24788ad7dfa6619c205c660e0be03281ffc Mon Sep 17 00:00:00 2001 From: Neelkamal Semwal Date: Fri, 12 Mar 2021 10:15:49 +0530 Subject: [PATCH 14/17] encoder: fix invalid free of raw buffers Return current input buffer as buffer to be freed in case of errors that are seen before picking up the input buffer to be from the input queue. Once a buffer is picked up from the queue, that is returned as the buffer to be freed. There is no need to return a buffer from ps_proc context Bug: 180643802 Test: poc in the bug description Test: atest CtsMediaV2TestCases:CodecEncoderTest Test: atest VtsHalMediaC2V1_0TargetVideoEncTest Change-Id: I1671ca1e82f522004d1f070df89b256b856f75b8 --- encoder/ih264e_encode.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/encoder/ih264e_encode.c b/encoder/ih264e_encode.c index fb37765..fe23841 100644 --- a/encoder/ih264e_encode.c +++ b/encoder/ih264e_encode.c @@ -228,6 +228,9 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) ps_video_encode_op->s_ive_op.output_present = 0; ps_video_encode_op->s_ive_op.dump_recon = 0; ps_video_encode_op->s_ive_op.u4_encoded_frame_type = IV_NA_FRAME; + /* By default set the current input buffer as the buffer to be freed */ + /* This will later be updated to the actual input that gets encoded */ + ps_video_encode_op->s_ive_op.s_inp_buf = ps_video_encode_ip->s_ive_ip.s_inp_buf; /* Check for output memory allocation size */ if (ps_video_encode_ip->s_ive_ip.s_out_buf.u4_bufsize < MIN_STREAM_SIZE) @@ -474,6 +477,9 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) s_out_buf.u4_is_last = s_inp_buf.u4_is_last; ps_video_encode_op->s_ive_op.u4_is_last = s_inp_buf.u4_is_last; + /* Send the input to application so that it can free it */ + ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; + /* Only encode if the current frame is not pre-encode skip */ if (!i4_rc_pre_enc_skip && s_inp_buf.s_raw_buf.apv_bufs[0]) { @@ -774,12 +780,6 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) } else { - /* proc ctxt base idx */ - WORD32 proc_ctxt_select = ctxt_sel * MAX_PROCESS_THREADS; - - /* proc ctxt */ - process_ctxt_t *ps_proc = &ps_codec->as_process[proc_ctxt_select]; - /* receive output back from codec */ s_out_buf = ps_codec->as_out_buf[ctxt_sel]; @@ -790,18 +790,11 @@ WORD32 ih264e_encode(iv_obj_t *ps_codec_obj, void *pv_api_ip, void *pv_api_op) ps_video_encode_op->s_ive_op.u4_timestamp_low = 0; ps_video_encode_op->s_ive_op.u4_timestamp_high = 0; - /* receive input back from codec and send it to app */ - s_inp_buf = ps_proc->s_inp_buf; - ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; - ps_video_encode_op->s_ive_op.u4_encoded_frame_type = IV_NA_FRAME; } - /* Send the input to encoder so that it can free it if possible */ ps_video_encode_op->s_ive_op.s_out_buf = s_out_buf.s_bits_buf; - ps_video_encode_op->s_ive_op.s_inp_buf = s_inp_buf.s_raw_buf; - if (1 == s_inp_buf.u4_is_last) { From 646a58ccf1cfbf191d0a3e247664f30c48fdee4d Mon Sep 17 00:00:00 2001 From: Neelkamal Semwal Date: Thu, 1 Apr 2021 12:52:14 +0530 Subject: [PATCH 15/17] encoder: Use intermediate buffer for accessing last MB row few SIMD modules read few more bytes from the input buffer at the end of frame. To avoid OOB read, intermediate buffer is used while processing last MB row. Bug: 180505809 Test: poc in bug Test: atest CtsMediaTestCases:VideoEncoderTest Test: atest CtsMediaV2TestCases:CodecEncoderTest Test: atest VtsHalMediaC2V1_0TargetVideoEncTest Change-Id: I11ca65937c7dfaf623f3535c02158ceec0dcc6ee --- encoder/ih264e_process.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/encoder/ih264e_process.c b/encoder/ih264e_process.c index 289053f..bf4c1e7 100644 --- a/encoder/ih264e_process.c +++ b/encoder/ih264e_process.c @@ -1259,7 +1259,8 @@ IH264E_ERROR_T ih264e_init_proc_ctxt(process_ctxt_t *ps_proc) /* init buffer pointers */ convert_uv_only = 1; if (u4_pad_bottom_sz || u4_pad_right_sz || - ps_codec->s_cfg.e_inp_color_fmt == IV_YUV_422ILE) + ps_codec->s_cfg.e_inp_color_fmt == IV_YUV_422ILE || + ps_proc->i4_mb_y == (ps_proc->i4_ht_mbs - 1)) { if (ps_proc->i4_mb_y == ps_proc->i4_ht_mbs - 1) u2_num_rows = (UWORD16) MB_SIZE - u4_pad_bottom_sz; @@ -1311,7 +1312,7 @@ IH264E_ERROR_T ih264e_init_proc_ctxt(process_ctxt_t *ps_proc) case IV_YUV_420SP_UV: case IV_YUV_420SP_VU: /* In case of 420 semi-planar input, copy last few rows to intermediate - buffer as chroma trans functions access one extra byte due to interleaved input. + buffer as few SIMD functions access upto 16 more bytes. This data will be padded if required */ if (ps_proc->i4_mb_y == (ps_proc->i4_ht_mbs - 1) || u4_pad_bottom_sz || u4_pad_right_sz) { @@ -1324,16 +1325,13 @@ IH264E_ERROR_T ih264e_init_proc_ctxt(process_ctxt_t *ps_proc) pu1_dst = ps_proc->pu1_src_buf_luma; - /* If padding is required, we always copy luma, if padding isn't required we never copy luma. */ - if (u4_pad_bottom_sz || u4_pad_right_sz) { - if (ps_proc->i4_mb_y == (ps_proc->i4_ht_mbs - 1)) - num_rows = MB_SIZE - u4_pad_bottom_sz; - for (i = 0; i < num_rows; i++) - { - memcpy(pu1_dst, pu1_src, ps_codec->s_cfg.u4_wd); - pu1_src += ps_proc->s_inp_buf.s_raw_buf.au4_strd[0]; - pu1_dst += ps_proc->i4_src_strd; - } + if (ps_proc->i4_mb_y == (ps_proc->i4_ht_mbs - 1)) + num_rows = MB_SIZE - u4_pad_bottom_sz; + for (i = 0; i < num_rows; i++) + { + memcpy(pu1_dst, pu1_src, ps_codec->s_cfg.u4_wd); + pu1_src += ps_proc->s_inp_buf.s_raw_buf.au4_strd[0]; + pu1_dst += ps_proc->i4_src_strd; } pu1_src = (UWORD8 *)ps_proc->s_inp_buf.s_raw_buf.apv_bufs[1] + (i4_mb_x * BLK8x8SIZE) + ps_proc->s_inp_buf.s_raw_buf.au4_strd[1] * (i4_mb_y * BLK8x8SIZE); @@ -1414,6 +1412,18 @@ IH264E_ERROR_T ih264e_init_proc_ctxt(process_ctxt_t *ps_proc) ps_proc->i4_src_chroma_strd, u4_pad_ht / 2, u4_pad_wd); } + if (ps_proc->i4_mb_y && ps_proc->i4_mb_y == ps_proc->i4_ht_mbs - 1) { + UWORD8 *pu1_src = (UWORD8 *)ps_proc->s_inp_buf.s_raw_buf.apv_bufs[0] + + ps_proc->s_inp_buf.s_raw_buf.au4_strd[0] * (i4_mb_y * MB_SIZE) - + ps_proc->s_inp_buf.s_raw_buf.au4_strd[0]; + UWORD8 *pu1_dst = ps_proc->pu1_src_buf_luma - ps_proc->i4_src_strd; + memcpy(pu1_dst, pu1_src, ps_codec->s_cfg.u4_wd); + if (u4_pad_right_sz && (ps_proc->i4_mb_x == 0)) { + pu1_dst += ps_codec->s_cfg.u4_disp_wd; + memset(pu1_dst, pu1_dst[-1], u4_pad_right_sz); + } + } + /* pad bottom edge */ if (u4_pad_bottom_sz && (ps_proc->i4_mb_y == ps_proc->i4_ht_mbs - 1) && ps_proc->i4_mb_x == 0) { From 4a22b5d1055ae2c3cf5699579c2c363a58ce81d0 Mon Sep 17 00:00:00 2001 From: Rakesh Kumar Date: Wed, 28 Apr 2021 23:44:50 +0530 Subject: [PATCH 16/17] Decoder: Update check for increment u2_cur_slice_num Increment u2_cur_slice_num only if current slice had atleast one MB of memory left. Test: clusterfuzz generated poc in bug Bug: b/182152757 Bug: b/179938345 Bug: b/185112718 Change-Id: Ic5eb07e961bccb7fde954bcfd791fd879804e335 --- decoder/ih264d_parse_slice.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/decoder/ih264d_parse_slice.c b/decoder/ih264d_parse_slice.c index f4413c7..266c69b 100644 --- a/decoder/ih264d_parse_slice.c +++ b/decoder/ih264d_parse_slice.c @@ -1435,17 +1435,20 @@ WORD32 ih264d_parse_decode_slice(UWORD8 u1_is_idr_slice, i1_is_end_of_poc = 0; } - if (ps_dec->u4_first_slice_in_pic == 0) + /* Increment only if the current slice has atleast 1 more MB */ + if (ps_dec->u4_first_slice_in_pic == 0 && + (ps_dec->ps_parse_cur_slice->u4_first_mb_in_slice < + (UWORD32)(ps_dec->u2_total_mbs_coded >> ps_dec->ps_cur_slice->u1_mbaff_frame_flag))) { ps_dec->ps_parse_cur_slice++; ps_dec->u2_cur_slice_num++; + // in the case of single core increment ps_decode_cur_slice + if(ps_dec->u1_separate_parse == 0) + { + ps_dec->ps_decode_cur_slice++; + } } - // in the case of single core increment ps_decode_cur_slice - if((ps_dec->u1_separate_parse == 0) && (ps_dec->u4_first_slice_in_pic == 0)) - { - ps_dec->ps_decode_cur_slice++; - } ps_dec->u1_slice_header_done = 0; From a88e0683a420d7ee9aa4b6f41f94cb8dc0c5e040 Mon Sep 17 00:00:00 2001 From: Rakesh Kumar Date: Wed, 28 Apr 2021 23:44:50 +0530 Subject: [PATCH 17/17] Decoder: Update check for increment u2_cur_slice_num Increment u2_cur_slice_num only if current slice had atleast one MB of memory left. Test: clusterfuzz generated poc in bug Bug: b/182152757 Bug: b/179938345 Bug: b/185112718 Change-Id: Ic5eb07e961bccb7fde954bcfd791fd879804e335 --- decoder/ih264d_parse_slice.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/decoder/ih264d_parse_slice.c b/decoder/ih264d_parse_slice.c index cf2dda9..ffe7f2b 100644 --- a/decoder/ih264d_parse_slice.c +++ b/decoder/ih264d_parse_slice.c @@ -1476,17 +1476,20 @@ WORD32 ih264d_parse_decode_slice(UWORD8 u1_is_idr_slice, i1_is_end_of_poc = 0; } - if (ps_dec->u4_first_slice_in_pic == 0) + /* Increment only if the current slice has atleast 1 more MB */ + if (ps_dec->u4_first_slice_in_pic == 0 && + (ps_dec->ps_parse_cur_slice->u4_first_mb_in_slice < + (UWORD32)(ps_dec->u2_total_mbs_coded >> ps_dec->ps_cur_slice->u1_mbaff_frame_flag))) { ps_dec->ps_parse_cur_slice++; ps_dec->u2_cur_slice_num++; + // in the case of single core increment ps_decode_cur_slice + if(ps_dec->u1_separate_parse == 0) + { + ps_dec->ps_decode_cur_slice++; + } } - // in the case of single core increment ps_decode_cur_slice - if((ps_dec->u1_separate_parse == 0) && (ps_dec->u4_first_slice_in_pic == 0)) - { - ps_dec->ps_decode_cur_slice++; - } ps_dec->u1_slice_header_done = 0;