Skip to content

Commit

Permalink
[Encode] Fix some coverity issues exposed in encode
Browse files Browse the repository at this point in the history 
Fix some encode coverity issues: 11 Unchecked return value; 5 Dead code; 4 Resource leak; 3 Modulo by zero; 1 Argument cannot be negative; 1 Out-of-bounds write

Signed-off-by: Chen, Bohan <[email protected]>
  • Loading branch information
@Bossonor
Bossonor committed Nov 15, 2023
1 parent 1258c71 commit b06fbc7
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 93 deletions.
28 changes: 17 additions & 11 deletions encode/av1encode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,8 @@ static void process_cmdline(int argc, char *argv[])
else {
struct stat tmp;

fstat(fileno(srcyuv_fp), &tmp);
VAStatus va_status = fstat(fileno(srcyuv_fp), &tmp);
CHECK_VASTATUS(va_status, "fstat");
srcyuv_frames = tmp.st_size / (ips.width * ips.height * 1.5);
printf("Source YUV file %s with %llu frames\n", ips.srcyuv, srcyuv_frames);

Expand Down Expand Up @@ -1856,12 +1857,6 @@ pack_render_size(bitstream* bs)
uint32_t render_and_frame_size_different = 0;

put_ui(bs, render_and_frame_size_different, 1);//render_and_frame_size_different

if (render_and_frame_size_different == 1)
{
put_ui(bs, fh.RenderWidth - 1, 16);//render_width_minus_1
put_ui(bs, fh.RenderHeight - 1, 16);//render_height_minus_1
}
}

static void
Expand Down Expand Up @@ -2554,25 +2549,34 @@ static int save_codeddata(unsigned long long display_order, unsigned long long e
}

long frame_end = ftell(coded_fp);
if (frame_start < 0 || frame_end < 0){
printf("function ftell returns a negative offset\n");
exit(1);
}

vaUnmapBuffer(va_dpy, coded_buf[display_order % SURFACE_NUM]);

if(encode_order == 0)
{
//first frame
unsigned int ivf_size = coded_size - 32 - 12;
fseek(coded_fp, frame_start + 32, SEEK_SET);
va_status = fseek(coded_fp, frame_start + 32, SEEK_SET);
CHECK_VASTATUS(va_status, "fseek");
fwrite(&ivf_size, 4, 1, coded_fp);
fwrite(&display_order, 8, 1, coded_fp);
fseek(coded_fp, frame_end, SEEK_SET);
va_status = fseek(coded_fp, frame_end, SEEK_SET);
CHECK_VASTATUS(va_status, "fseek");
}
else
{
//other frames
unsigned int ivf_size = coded_size - 12;
fseek(coded_fp, frame_start, SEEK_SET);
va_status = fseek(coded_fp, frame_start, SEEK_SET);
CHECK_VASTATUS(va_status, "fseek");
fwrite(&ivf_size, 4, 1, coded_fp);
fwrite(&display_order, 8, 1, coded_fp);
fseek(coded_fp, frame_end, SEEK_SET);
va_status = fseek(coded_fp, frame_end, SEEK_SET);
CHECK_VASTATUS(va_status, "fseek");
}

printf("\n "); /* return back to startpoint */
Expand Down Expand Up @@ -2899,6 +2903,8 @@ static int calc_PSNR(double *psnr)
srcyuv_ptr = mmap(0, fourM, PROT_READ, MAP_SHARED, fileno(srcyuv_fp), i);
recyuv_ptr = mmap(0, fourM, PROT_READ, MAP_SHARED, fileno(recyuv_fp), i);
if ((srcyuv_ptr == MAP_FAILED) || (recyuv_ptr == MAP_FAILED)) {
free(srcyuv_ptr);
free(recyuv_ptr);
printf("Failed to mmap YUV files\n");
return 1;
}
Expand Down
10 changes: 6 additions & 4 deletions encode/avcenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ static int begin_picture(FILE *yuv_fp, int frame_num, int display_num, int slice
/* hrd parameter */
VAEncMiscParameterBuffer *misc_param;
VAEncMiscParameterHRD *misc_hrd_param;
vaCreateBuffer(va_dpy,
va_status = vaCreateBuffer(va_dpy,
avcenc_context.context_id,
VAEncMiscParameterBufferType,
sizeof(VAEncMiscParameterBuffer) + sizeof(VAEncMiscParameterRateControl),
Expand Down Expand Up @@ -878,13 +878,14 @@ static int begin_picture(FILE *yuv_fp, int frame_num, int display_num, int slice
VAEncMiscParameterBufferROI *misc_roi_param;

int roi_num = 1;
vaCreateBuffer(va_dpy,
va_status = vaCreateBuffer(va_dpy,
avcenc_context.context_id,
VAEncMiscParameterBufferType,
sizeof(VAEncMiscParameterBuffer) + sizeof(VAEncMiscParameterBufferROI) + roi_num * sizeof(VAEncROI),
1,
NULL,
&avcenc_context.misc_parameter_roi_buf_id);
CHECK_VASTATUS(va_status, "vaCreateBuffer");
vaMapBuffer(va_dpy,
avcenc_context.misc_parameter_roi_buf_id,
(void **)&misc_param);
Expand Down Expand Up @@ -1792,7 +1793,8 @@ encode_picture(FILE *yuv_fp, FILE *avc_fp,
index = SID_INPUT_PICTURE_0;
if (next_display_num >= frame_number)
next_display_num = frame_number - 1;
fseeko(yuv_fp, (off_t)frame_size * next_display_num, SEEK_SET);
va_status = fseeko(yuv_fp, (off_t)frame_size * next_display_num, SEEK_SET);
CHECK_VASTATUS(va_status, "fseeko");

avcenc_context.upload_thread_param.yuv_fp = yuv_fp;
avcenc_context.upload_thread_param.surface_id = surface_ids[index];
Expand Down Expand Up @@ -2108,7 +2110,7 @@ int main(int argc, char *argv[])
file_size = ftello(yuv_fp);
frame_size = picture_width * picture_height + ((picture_width * picture_height) >> 1) ;

if ((file_size < frame_size) || (file_size % frame_size)) {
if ((file_size < frame_size) || ((frame_size != 0) && (file_size % frame_size))) {
fclose(yuv_fp);
printf("The YUV file's size is not correct\n");
return -1;
Expand Down
73 changes: 21 additions & 52 deletions encode/hevcencode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1280,8 +1280,6 @@ static void sliceHeader_rbsp(
{
uint8_t nal_unit_type = NALU_TRAIL_R;
int gop_ref_distance = ip_period;
int incomplete_mini_gop = 0;
int p_slice_flag = 1;
int i = 0;

put_ui(bs, slice_header->first_slice_segment_in_pic_flag, 1);
Expand Down Expand Up @@ -1337,51 +1335,21 @@ static void sliceHeader_rbsp(
if (1 == gop_ref_distance) {
put_ue(bs, 0 /*delta_poc_s0_minus1*/);
} else {
if (incomplete_mini_gop) {
if (frame_cnt_in_gop % gop_ref_distance > i) {
put_ue(bs, 0 /*delta_poc_s0_minus1*/);
} else {
int DeltaPoc = -(int)(gop_ref_distance);
put_ue(bs, prev - DeltaPoc - 1 /*delta_poc_s0_minus1*/);
}
// For Non-BPyramid GOP i.e B0 type
if (num_active_ref_p > 1) {
// DeltaPOC Equals NumB
int DeltaPoc = -(int)(gop_ref_distance);
put_ue(bs, prev - DeltaPoc - 1 /*delta_poc_s0_minus1*/);
} else {
// For Non-BPyramid GOP i.e B0 type
if (num_active_ref_p > 1) {
// MultiRef Case
if (p_slice_flag) {
// DeltaPOC Equals NumB
int DeltaPoc = -(int)(gop_ref_distance);
put_ue(bs, prev - DeltaPoc - 1 /*delta_poc_s0_minus1*/);
} else {
// for normal B
if (frame_cnt_in_gop < gop_ref_distance) {
if (0 == i) {
int DeltaPoc = -(int)(frame_cnt_in_gop);
put_ue(bs, prev - DeltaPoc - 1 /*delta_poc_s0_minus1*/);
}
} else if (frame_cnt_in_gop > gop_ref_distance) {
if (0 == i) {
//Need % to wraparound the delta poc, to avoid corruption caused on POC=5 with GOP (29,2) and 4 refs
int DeltaPoc = -(int)((frame_cnt_in_gop - gop_ref_distance) % gop_ref_distance);
put_ue(bs, prev - DeltaPoc - 1 /*delta_poc_s0_minus1*/);
} else if (1 <= i) {
int DeltaPoc = -(int)(gop_ref_distance);
put_ue(bs, prev - DeltaPoc - 1 /*delta_poc_s0_minus1*/);
}
}
}
} else {
// the big 'if' wraps here is -
// if (!slice_header->short_term_ref_pic_set_sps_flag)
// From the Teddi logic, the short_term_ref_pic_set_sps_flag only can be '0'
// either for B-Prymid or first several frames in a GOP in multi-ref cases
// when there are not enough backward refs.
// So though there are really some codes under this 'else'in Teddi, don't
// want to introduce them in MEA to avoid confusion, and put an assert
// here to guard that there is new case we need handle in the future.
assert(0);

}
// the big 'if' wraps here is -
// if (!slice_header->short_term_ref_pic_set_sps_flag)
// From the Teddi logic, the short_term_ref_pic_set_sps_flag only can be '0'
// either for B-Prymid or first several frames in a GOP in multi-ref cases
// when there are not enough backward refs.
// So though there are really some codes under this 'else'in Teddi, don't
// want to introduce them in MEA to avoid confusion, and put an assert
// here to guard that there is new case we need handle in the future.
assert(0);
}
}
put_ui(bs, 1 /*used_by_curr_pic_s0_flag*/, 1);
Expand Down Expand Up @@ -1528,11 +1496,6 @@ static void sliceHeader_rbsp(
int slice_header_extension_length = 0;

put_ue(bs, slice_header_extension_length);

for (i = 0; i < slice_header_extension_length; i++) {
int slice_header_extension_data_byte = 0;
put_ui(bs, slice_header_extension_data_byte, 8);
}
}
}

Expand Down Expand Up @@ -1844,6 +1807,8 @@ static int process_cmdline(int argc, char *argv[])
frame_rate = atoi(optarg);
break;
case 'o':
if (coded_fn)
free(coded_fn);
coded_fn = strdup(optarg);
break;
case 0:
Expand Down Expand Up @@ -1875,9 +1840,13 @@ static int process_cmdline(int argc, char *argv[])
}
break;
case 9:
if (srcyuv_fn)
free(srcyuv_fn);
srcyuv_fn = strdup(optarg);
break;
case 10:
if (recyuv_fn)
free(recyuv_fn);
recyuv_fn = strdup(optarg);
break;
case 11:
Expand Down Expand Up @@ -2511,7 +2480,7 @@ static int render_picture(struct PicParamSet *pps)
int i = 0;

memcpy(pic_param.reference_frames, ReferenceFrames, numShortTerm * sizeof(VAPictureHEVC));
for (i = numShortTerm; i < SURFACE_NUM; i++) {
for (i = numShortTerm; i < SURFACE_NUM - 1; i++) {
pic_param.reference_frames[i].picture_id = VA_INVALID_SURFACE;
pic_param.reference_frames[i].flags = VA_PICTURE_HEVC_INVALID;
}
Expand Down
2 changes: 1 addition & 1 deletion encode/jpegenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -976,7 +976,7 @@ int main(int argc, char *argv[])
fseeko(yuv_fp, (off_t)0, SEEK_END);
file_size = ftello(yuv_fp);

if ((file_size < frame_size) || (file_size % frame_size)) {
if ((file_size < frame_size) || ((frame_size != 0) && (file_size % frame_size))) {
fclose(yuv_fp);
printf("The YUV file's size is not correct: file_size=%zd, frame_size=%d\n", file_size, frame_size);
return -1;
Expand Down
3 changes: 2 additions & 1 deletion encode/mpeg2vaenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1382,7 +1382,8 @@ encode_picture(struct mpeg2enc_context *ctx,
if (next_display_order >= ctx->num_pictures)
next_display_order = ctx->num_pictures - 1;

fseek(ctx->ifp, ctx->frame_size * next_display_order, SEEK_SET);
va_status = fseek(ctx->ifp, ctx->frame_size * next_display_order, SEEK_SET);
CHECK_VASTATUS(va_status, "fseek");
ctx->upload_thread_value = pthread_create(&ctx->upload_thread_id,
NULL,
upload_yuv_to_surface,
Expand Down
3 changes: 2 additions & 1 deletion encode/svctenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1159,7 +1159,8 @@ upload_task(struct svcenc_context *ctx, unsigned int display_order, int surface)
int y_size = ctx->width * ctx->height;
int u_size = (ctx->width >> 1) * (ctx->height >> 1);

fseek(ctx->ifp, ctx->frame_size * display_order, SEEK_SET);
va_status = fseek(ctx->ifp, ctx->frame_size * display_order, SEEK_SET);
CHECK_VASTATUS(va_status, "fseek");

do {
n_items = fread(ctx->frame_data_buffer, ctx->frame_size, 1, ctx->ifp);
Expand Down
21 changes: 5 additions & 16 deletions encode/vp8enc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,6 @@ void vp8enc_create_EncoderPipe()
VAEntrypoint entrypoints[5];
int num_entrypoints;
int i;
bool entrypoint_found;
VAConfigAttrib conf_attrib[2];
VASurfaceAttrib surface_attrib;
int major_ver, minor_ver;
Expand All @@ -686,17 +685,6 @@ void vp8enc_create_EncoderPipe()
vaQueryConfigEntrypoints(vaapi_context.display, vaapi_context.profile, entrypoints,
&num_entrypoints);

entrypoint_found = true;
for (i = 0; i < num_entrypoints; i++) {
if (entrypoints[i] == settings.vaapi_entry_point)
entrypoint_found = true;
}

if (entrypoint_found == false) {
fprintf(stderr, "Error: VAEntrypoint not found!\n");
assert(0);
}

/* find out the format for the render target, and rate control mode */
conf_attrib[0].type = VAConfigAttribRTFormat;
conf_attrib[1].type = VAConfigAttribRateControl;
Expand Down Expand Up @@ -835,7 +823,8 @@ vp8enc_store_coded_buffer(FILE *vp8_fp, uint64_t timestamp)
size_t vp8enc_get_FileSize(FILE *fp)
{
struct stat st;
fstat(fileno(fp), &st);
VAStatus va_status = fstat(fileno(fp), &st);
CHECK_VASTATUS(va_status, "fstat");
return st.st_size;
}

Expand Down Expand Up @@ -882,7 +871,7 @@ int vp8enc_prepare_buffers(int frame_type)


/* hrd parameter */
vaCreateBuffer(vaapi_context.display,
va_status = vaCreateBuffer(vaapi_context.display,
vaapi_context.context_id,
VAEncMiscParameterBufferType,
sizeof(vaapi_context.hrd_param), 1, &vaapi_context.hrd_param,
Expand All @@ -905,7 +894,7 @@ int vp8enc_prepare_buffers(int frame_type)
num_buffers++;
/* Create the Misc FR/RC buffer under non-CQP mode */
if (settings.rc_mode != VA_RC_CQP && frame_type == KEY_FRAME) {
vaCreateBuffer(vaapi_context.display,
va_status = vaCreateBuffer(vaapi_context.display,
vaapi_context.context_id,
VAEncMiscParameterBufferType,
sizeof(vaapi_context.frame_rate_param), 1, &vaapi_context.frame_rate_param,
Expand All @@ -915,7 +904,7 @@ int vp8enc_prepare_buffers(int frame_type)
va_buffers ++;
num_buffers++;

vaCreateBuffer(vaapi_context.display,
va_status = vaCreateBuffer(vaapi_context.display,
vaapi_context.context_id,
VAEncMiscParameterBufferType,
sizeof(vaapi_context.rate_control_param), 1, &vaapi_context.rate_control_param,
Expand Down
17 changes: 10 additions & 7 deletions encode/vp9enc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ vp9enc_begin_picture(FILE *yuv_fp, int frame_num, int frame_type)
/* hrd parameter */
VAEncMiscParameterBuffer *misc_param;
VAEncMiscParameterHRD *misc_hrd_param;
vaCreateBuffer(va_dpy,
va_status = vaCreateBuffer(va_dpy,
vp9enc_context.context_id,
VAEncMiscParameterBufferType,
sizeof(VAEncMiscParameterBuffer) + sizeof(VAEncMiscParameterRateControl),
Expand Down Expand Up @@ -938,7 +938,7 @@ vp9enc_begin_picture(FILE *yuv_fp, int frame_num, int frame_type)
VAEncMiscParameterFrameRate *misc_fr;
VAEncMiscParameterRateControl *misc_rc;

vaCreateBuffer(va_dpy,
va_status = vaCreateBuffer(va_dpy,
vp9enc_context.context_id,
VAEncMiscParameterBufferType,
sizeof(VAEncMiscParameterBuffer) + sizeof(VAEncMiscParameterFrameRate),
Expand All @@ -955,7 +955,7 @@ vp9enc_begin_picture(FILE *yuv_fp, int frame_num, int frame_type)
misc_fr->framerate = frame_rate;
vaUnmapBuffer(va_dpy, vp9enc_context.misc_fr_buf_id);

vaCreateBuffer(va_dpy,
va_status = vaCreateBuffer(va_dpy,
vp9enc_context.context_id,
VAEncMiscParameterBufferType,
sizeof(VAEncMiscParameterBuffer) + sizeof(VAEncMiscParameterRateControl),
Expand Down Expand Up @@ -1182,7 +1182,8 @@ vp9enc_encode_picture(FILE *yuv_fp, FILE *vp9_fp,
else
index = SID_INPUT_PICTURE_0;

fseeko(yuv_fp, (off_t)frame_size * next_enc_frame, SEEK_SET);
va_status = fseeko(yuv_fp, (off_t)frame_size * next_enc_frame, SEEK_SET);
CHECK_VASTATUS(va_status, "fseeko");

vp9enc_context.upload_thread_param.yuv_fp = yuv_fp;
vp9enc_context.upload_thread_param.surface_id = surface_ids[index];
Expand Down Expand Up @@ -1227,17 +1228,19 @@ vp9enc_encode_picture(FILE *yuv_fp, FILE *vp9_fp,
packed_header_param_buffer.bit_length = raw_data_length * 8;
packed_header_param_buffer.has_emulation_bytes = 0;

vaCreateBuffer(va_dpy,
va_status = vaCreateBuffer(va_dpy,
vp9enc_context.context_id,
VAEncPackedHeaderParameterBufferType,
sizeof(packed_header_param_buffer), 1, &packed_header_param_buffer,
&vp9enc_context.raw_data_header_buf_id);
CHECK_VASTATUS(va_status, "vaCreateBuffer");

vaCreateBuffer(va_dpy,
va_status = vaCreateBuffer(va_dpy,
vp9enc_context.context_id,
VAEncPackedHeaderDataBufferType,
raw_data_length, 1, raw_data,
&vp9enc_context.raw_data_buf_id);
CHECK_VASTATUS(va_status, "vaCreateBuffer");
}

vp9enc_create_picture_parameter_buf();
Expand Down Expand Up @@ -1531,7 +1534,7 @@ main(int argc, char *argv[])
file_size = ftello(yuv_fp);
frame_size = picture_width * picture_height + ((picture_width * picture_height) >> 1) ;

if ((file_size < frame_size) || (file_size % frame_size)) {
if ((file_size < frame_size) || ((frame_size != 0) && (file_size % frame_size))) {
fclose(yuv_fp);
printf("The YUV file's size is not correct\n");
return -1;
Expand Down

0 comments on commit b06fbc7

Please sign in to comment.