On 27/02/2019 16:12, Guo, Yejun wrote: > Signed-off-by: Guo, Yejun <yejun....@intel.com> > --- > libavcodec/libvpxenc.c | 132 > +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 132 insertions(+)
Do these APIs exist for all supported libvpx versions? > + if (!sd) { > + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ACTIVEMAP, > &active_map)) { > + log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP codec > control.\n"); > + return AVERROR_INVALIDDATA; > + } Why do this stuff at all if no ROIs have been set? > + memset(active_map.active_map, 1, active_map.rows * active_map.cols); Nit: Maybe a comment about what '1' is here. > + > + /* segment id 0 in roi_map is reserved for the areas not covered by > AVRegionOfInterest. > + * segment id 0 in roi_map is also for the areas with > AVRegionOfInterest.qoffset near 0. > + */ > + segment_id = 0; > + segment_mapping[zero_delta_q + MAX_DELTA_Q] = segment_id + 1; > + segment_id++; This series of ops seems weirdly redundant separately? > + memset(&roi_map, 0, sizeof(roi_map)); Can't zero-init? > + av_free(active_map.active_map); > + av_log(avctx, AV_LOG_ERROR, > + "roi_map alloc (%d bytes) failed.\n", > + roi_map.rows * roi_map.cols); Here, and elsewhere: We don't write how many bytes failed, generally. > + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ROI_MAP, &roi_map)) { > + av_free(active_map.active_map); > + av_free(roi_map.roi_map); > + log_encoder_error(avctx, "Failed to set VP8E_SET_ROI_MAP codec > control.\n"); > + return AVERROR_INVALIDDATA; > + } > + > + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ACTIVEMAP, &active_map)) { > + av_free(active_map.active_map); > + av_free(roi_map.roi_map); > + log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP codec > control.\n"); > + return AVERROR_INVALIDDATA; > + } > + > + av_free(active_map.active_map); > + av_free(roi_map.roi_map); With the amount of failure areas in this function, is it reasonable to roll it into one goto fail or something? > + if (avctx->codec_id == AV_CODEC_ID_VP8) > + vp8_encode_set_roi(avctx, frame); The API only exists for VP8, or is this due to different quant scales or something? As an aside, I must say, libvpx's ROI API is real ugly. Cheers, - Derek _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel