[FFmpeg-devel] [PATCH V5] avcodec/libvpxenc: add ROI-based encoding support for VP8/VP9 support

[James Zern]

Hi,

On Thu, Aug 22, 2019 at 5:56 PM Guo, Yejun <yejun.guo at intel.com> wrote:
>
> example command line to verify it:
> ./ffmpeg -i input.stream -vf addroi=0:0:iw/3:ih/3:-0.8 -c:v libvpx -b:v 2M tmp.webm
>
> Signed-off-by: Guo, Yejun <yejun.guo at intel.com>
> ---
>  libavcodec/libvpxenc.c | 194 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 194 insertions(+)
>

Looks OK overall, just a few minor comments and you should bump the
micro version number for this change.

> [...]
> +
> +            segment_mapping[mapping_index] = segment_id + 1;
> +            roi_map->delta_q[segment_id] = delta_q;
> +            segment_id++;
> +        }
> +    }
> +
> +

This line can go.

> [...]
> +
> +    av_assert0(!roi_supported);
> +    if (!ctx->roi_warned) {
> +        ctx->roi_warned = 1;
> +        av_log(avctx, AV_LOG_WARNING, "ROI is not supported, please upgrade libvpx to version >= 1.8.1, "
> +                                      "and you might need to build ffmpeg again.\n");

This could be "...to version >= 1.8.1. You may need to rebuild ffmpeg.\n"

> [...]
> +
> +        if (sd) {
> +            if (avctx->codec_id == AV_CODEC_ID_VP8)
> +                vp8_encode_set_roi(avctx, frame->width, frame->height, sd);
> +            else if (avctx->codec_id == AV_CODEC_ID_VP9)

This is the only other option, it can just be 'else'. Other checks in
the code assume this already.

> +                vp9_encode_set_roi(avctx, frame->width, frame->height, sd);
> +        }
>      }