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 @XinfengZhang
Bossonor authored and XinfengZhang committed Nov 30, 2023
1 parent b0389f4 commit 505e7b3
Show file tree
Hide file tree
Showing 8 changed files with 100 additions and 97 deletions.
33 changes: 19 additions & 14 deletions encode/av1encode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
exit(1); \
}

#define CHECK_CONDITION(cod) \
if(!(cod)) \
#define CHECK_CONDITION(cond) \
if(!(cond)) \
{ \
fprintf(stderr, "Unexpected condition:%s:%d\n", __func__, __LINE__); \
fprintf(stderr, "Unexpected condition: %s:%d\n", __func__, __LINE__); \
exit(1); \
}

Expand Down Expand Up @@ -806,7 +806,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_CONDITION(ret == 0);
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 @@ -2539,6 +2534,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 @@ -2556,23 +2552,28 @@ 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]);

CHECK_CONDITION(frame_start >= 0 && frame_end >= 0);
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_CONDITION(ret == 0);
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_CONDITION(ret == 0);
}
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_CONDITION(ret == 0);
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_CONDITION(ret == 0);
}

printf("\n "); /* return back to startpoint */
Expand Down Expand Up @@ -2899,6 +2900,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
17 changes: 13 additions & 4 deletions encode/avcenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@
exit(1); \
}

#define CHECK_CONDITION(cond) \
if(!(cond)) \
{ \
fprintf(stderr, "Unexpected condition: %s:%d\n", __func__, __LINE__); \
exit(1); \
}

static VADisplay va_dpy;

static int picture_width, picture_width_in_mbs;
Expand Down Expand Up @@ -848,7 +855,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 +885,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 +1800,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_CONDITION(ret == 0);

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 +2117,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
10 changes: 9 additions & 1 deletion encode/mpeg2vaenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ enum {
exit(1); \
}

#define CHECK_CONDITION(cond) \
if(!(cond)) \
{ \
fprintf(stderr, "Unexpected condition: %s:%d\n", __func__, __LINE__); \
exit(1); \
}

static VAProfile mpeg2_va_profiles[] = {
VAProfileMPEG2Simple,
VAProfileMPEG2Main
Expand Down Expand Up @@ -1382,7 +1389,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_CONDITION(ret == 0);
ctx->upload_thread_value = pthread_create(&ctx->upload_thread_id,
NULL,
upload_yuv_to_surface,
Expand Down
10 changes: 9 additions & 1 deletion encode/svctenc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,13 @@
exit(1); \
}

#define CHECK_CONDITION(cond) \
if(!(cond)) \
{ \
fprintf(stderr, "Unexpected condition: %s:%d\n", __func__, __LINE__); \
exit(1); \
}

#define MAX_SLICES 32
#define MAX_LAYERS 4

Expand Down Expand Up @@ -1159,7 +1166,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_CONDITION(ret == 0);

do {
n_items = fread(ctx->frame_data_buffer, ctx->frame_size, 1, ctx->ifp);
Expand Down
28 changes: 11 additions & 17 deletions encode/vp8enc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,12 @@
}
#endif


#define CHECK_CONDITION(cond) \
if(!(cond)) \
{ \
fprintf(stderr, "Unexpected condition: %s:%d\n", __func__, __LINE__); \
exit(1); \
}

static const struct option long_opts[] = {
{"help", no_argument, NULL, 0 },
Expand Down Expand Up @@ -673,7 +678,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 +690,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 +828,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);
int ret = fstat(fileno(fp), &st);
CHECK_CONDITION(ret == 0);
return st.st_size;
}

Expand Down Expand Up @@ -882,7 +876,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 +899,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 +909,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
Loading

0 comments on commit 505e7b3

Please sign in to comment.