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 17, 2023
1 parent 1258c71 commit 9e97957
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 94 deletions.
38 changes: 27 additions & 11 deletions encode/av1encode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
exit(1); \
}

#define CHECK_FN(cod, func) \
if (!cod) { \
fprintf(stderr,"%s:%s (%d) failed, exit\n", __func__, func, __LINE__); \
exit(1); \
}

#define MIN(a, b) ((a)>(b)?(b):(a))
#define MAX(a, b) ((a)>(b)?(a):(b))

Expand Down Expand Up @@ -806,7 +812,8 @@ static void process_cmdline(int argc, char *argv[])
else {
struct stat tmp;

fstat(fileno(srcyuv_fp), &tmp);
int ret = fstat(fileno(srcyuv_fp), &tmp);
CHECK_FN(ret, "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 +1863,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 @@ -2539,6 +2540,7 @@ static int save_codeddata(unsigned long long display_order, unsigned long long e
{
VACodedBufferSegment *buf_list = NULL;
VAStatus va_status;
int ret;
unsigned int coded_size = 0;

va_status = vaMapBuffer(va_dpy, coded_buf[display_order % SURFACE_NUM], (void **)(&buf_list));
Expand All @@ -2554,25 +2556,35 @@ static int save_codeddata(unsigned long long display_order, unsigned long long e
}

long frame_end = ftell(coded_fp);

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

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

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

printf("\n "); /* return back to startpoint */
Expand Down Expand Up @@ -2899,6 +2911,10 @@ 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)) {
if (srcyuv_ptr)
munmap(srcyuv_ptr, fourM);
if (recyuv_ptr)
munmap(recyuv_ptr, fourM);
printf("Failed to mmap YUV files\n");
return 1;
}
Expand Down
16 changes: 12 additions & 4 deletions encode/avcenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@
exit(1); \
}

#define CHECK_FN(cod, func) \
if (!cod) { \
fprintf(stderr,"%s:%s (%d) failed, exit\n", __func__, func, __LINE__); \
exit(1); \
}

static VADisplay va_dpy;

static int picture_width, picture_width_in_mbs;
Expand Down Expand Up @@ -848,7 +854,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 +884,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 +1799,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);
ret = fseeko(yuv_fp, (off_t)frame_size * next_display_num, SEEK_SET);
CHECK_FN(ret, "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 +2116,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
9 changes: 8 additions & 1 deletion encode/mpeg2vaenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ enum {
exit(1); \
}

#define CHECK_FN(cod, func) \
if (!cod) { \
fprintf(stderr,"%s:%s (%d) failed, exit\n", __func__, func, __LINE__); \
exit(1); \
}

static VAProfile mpeg2_va_profiles[] = {
VAProfileMPEG2Simple,
VAProfileMPEG2Main
Expand Down Expand Up @@ -1382,7 +1388,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);
ret = fseek(ctx->ifp, ctx->frame_size * next_display_order, SEEK_SET);
CHECK_FN(ret, "fseek");
ctx->upload_thread_value = pthread_create(&ctx->upload_thread_id,
NULL,
upload_yuv_to_surface,
Expand Down
9 changes: 8 additions & 1 deletion encode/svctenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@
exit(1); \
}

#define CHECK_FN(cod, func) \
if (!cod) { \
fprintf(stderr,"%s:%s (%d) failed, exit\n", __func__, func, __LINE__); \
exit(1); \
}

#define MAX_SLICES 32
#define MAX_LAYERS 4

Expand Down Expand Up @@ -1159,7 +1165,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);
int ret = fseek(ctx->ifp, ctx->frame_size * display_order, SEEK_SET);
CHECK_FN(ret, "fseek");

do {
n_items = fread(ctx->frame_data_buffer, ctx->frame_size, 1, ctx->ifp);
Expand Down
Loading

0 comments on commit 9e97957

Please sign in to comment.