On 27/02/2019 16:12, Guo, Yejun wrote:
> Signed-off-by: Guo, Yejun <[email protected]>
> ---
> 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
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
