From 5b6ae020b6a972b67b59b3dbf7cf9cbd3140a80d Mon Sep 17 00:00:00 2001 From: Jerome Jiang Date: Wed, 17 Jan 2018 11:23:55 -0800 Subject: [PATCH] Fix crash invalid params for vp8 multres. Add test. Fix is from the patch in the issue. Release memories allocated before early exit. BUG=webm:1482 Change-Id: I64952af99c58241496e03fa55da09fd129a07c77 --- test/encode_api_test.cc | 72 +++++++++++++++++++++++++++++++++++++++++ vpx/src/vpx_encoder.c | 50 +++++++++++++++------------- 2 files changed, 100 insertions(+), 22 deletions(-) diff --git a/test/encode_api_test.cc b/test/encode_api_test.cc index 164db5a7b..13de53464 100644 --- a/test/encode_api_test.cc +++ b/test/encode_api_test.cc @@ -106,4 +106,76 @@ TEST(EncodeAPI, ImageSizeSetting) { } #endif +#if CONFIG_MULTI_RES_ENCODING +// Set up 2 spatial streams with 2 temporal layers per stream, and generate +// invalid configuration by setting the temporal layer rate allocation +// (ts_target_bitrate[]) to 0 for both layers. +TEST(EncodeAPI, VP8MultiResEncode) { + const int width = 1280; + const int height = 720; + const int width_down = width / 2; + const int height_down = height / 2; + const int target_bitrate = 1000; + const int framerate = 30; + vpx_codec_ctx_t enc[2]; + vpx_codec_enc_cfg_t cfg[2]; + vpx_rational_t dsf[2] = { { 2, 1 }, { 2, 1 } }; + + memset(enc, 0, sizeof(enc)); + + for (int i = 0; i < 2; i++) { + vpx_codec_enc_config_default(vpx_codec_vp8_cx(), &cfg[i], 0); + } + + /* Highest-resolution encoder settings */ + cfg[0].g_w = width; + cfg[0].g_h = height; + cfg[0].rc_dropframe_thresh = 0; + cfg[0].rc_end_usage = VPX_CBR; + cfg[0].rc_resize_allowed = 0; + cfg[0].rc_min_quantizer = 2; + cfg[0].rc_max_quantizer = 56; + cfg[0].rc_undershoot_pct = 100; + cfg[0].rc_overshoot_pct = 15; + cfg[0].rc_buf_initial_sz = 500; + cfg[0].rc_buf_optimal_sz = 600; + cfg[0].rc_buf_sz = 1000; + cfg[0].g_error_resilient = 1; /* Enable error resilient mode */ + cfg[0].g_lag_in_frames = 0; + + cfg[0].kf_mode = VPX_KF_AUTO; + cfg[0].kf_min_dist = 3000; + cfg[0].kf_max_dist = 3000; + + cfg[0].rc_target_bitrate = target_bitrate; /* Set target bitrate */ + cfg[0].g_timebase.num = 1; /* Set fps */ + cfg[0].g_timebase.den = framerate; + + memcpy(&cfg[1], &cfg[0], sizeof(cfg[0])); + cfg[1].rc_target_bitrate = 500; + cfg[1].g_w = width_down; + cfg[1].g_h = height_down; + + for (int i = 0; i < 2; i++) { + cfg[i].ts_number_layers = 2; + cfg[i].ts_periodicity = 2; + cfg[i].ts_rate_decimator[0] = 2; + cfg[i].ts_rate_decimator[1] = 1; + cfg[i].ts_layer_id[0] = 0; + cfg[i].ts_layer_id[1] = 1; + // Invalid parameters. + cfg[i].ts_target_bitrate[0] = 0; + cfg[i].ts_target_bitrate[1] = 0; + } + + EXPECT_EQ(VPX_CODEC_INVALID_PARAM, + vpx_codec_enc_init_multi(&enc[0], vpx_codec_vp8_cx(), &cfg[0], 2, 0, + &dsf[0])); + + for (int i = 0; i < 2; i++) { + vpx_codec_destroy(&enc[i]); + } +} +#endif + } // namespace diff --git a/vpx/src/vpx_encoder.c b/vpx/src/vpx_encoder.c index 4390cf7c8..a26204bc4 100644 --- a/vpx/src/vpx_encoder.c +++ b/vpx/src/vpx_encoder.c @@ -12,8 +12,11 @@ * \brief Provides the high level interface to wrap encoder algorithms. * */ +#include #include +#include #include +#include "vp8/common/blockd.h" #include "vpx_config.h" #include "vpx/internal/vpx_codec_internal.h" @@ -89,28 +92,27 @@ vpx_codec_err_t vpx_codec_enc_init_multi_ver( if (dsf->num < 1 || dsf->num > 4096 || dsf->den < 1 || dsf->den > dsf->num) { res = VPX_CODEC_INVALID_PARAM; - break; + } else { + mr_cfg.mr_low_res_mode_info = mem_loc; + mr_cfg.mr_total_resolutions = num_enc; + mr_cfg.mr_encoder_id = num_enc - 1 - i; + mr_cfg.mr_down_sampling_factor.num = dsf->num; + mr_cfg.mr_down_sampling_factor.den = dsf->den; + + /* Force Key-frame synchronization. Namely, encoder at higher + * resolution always use the same frame_type chosen by the + * lowest-resolution encoder. + */ + if (mr_cfg.mr_encoder_id) cfg->kf_mode = VPX_KF_DISABLED; + + ctx->iface = iface; + ctx->name = iface->name; + ctx->priv = NULL; + ctx->init_flags = flags; + ctx->config.enc = cfg; + res = ctx->iface->init(ctx, &mr_cfg); } - mr_cfg.mr_low_res_mode_info = mem_loc; - mr_cfg.mr_total_resolutions = num_enc; - mr_cfg.mr_encoder_id = num_enc - 1 - i; - mr_cfg.mr_down_sampling_factor.num = dsf->num; - mr_cfg.mr_down_sampling_factor.den = dsf->den; - - /* Force Key-frame synchronization. Namely, encoder at higher - * resolution always use the same frame_type chosen by the - * lowest-resolution encoder. - */ - if (mr_cfg.mr_encoder_id) cfg->kf_mode = VPX_KF_DISABLED; - - ctx->iface = iface; - ctx->name = iface->name; - ctx->priv = NULL; - ctx->init_flags = flags; - ctx->config.enc = cfg; - res = ctx->iface->init(ctx, &mr_cfg); - if (res) { const char *error_detail = ctx->priv ? ctx->priv->err_detail : NULL; /* Destroy current ctx */ @@ -124,10 +126,14 @@ vpx_codec_err_t vpx_codec_enc_init_multi_ver( vpx_codec_destroy(ctx); i--; } +#if CONFIG_MULTI_RES_ENCODING + assert(mem_loc); + free(((LOWER_RES_FRAME_INFO *)mem_loc)->mb_info); + free(mem_loc); +#endif + return SAVE_STATUS(ctx, res); } - if (res) break; - ctx++; cfg++; dsf++; -- 2.40.0