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

[Guo, Yejun]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of
> James Zern
> Sent: Wednesday, August 21, 2019 8:47 AM
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V3] avcodec/libvpxenc: add ROI-based
> encoding support for VP8/VP9 support
> 
> Hi,
> 
> On Tue, Aug 13, 2019 at 8:29 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 | 197
> +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 197 insertions(+)
> >
> > [...]
> >
> > +
> > +    memset(roi_map, 0, sizeof(*roi_map));
> > +
> > +    /* 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.
> > +     * (delta_q of segment id 0 is 0).
> > +     */
> > +    segment_mapping[MAX_DELTA_Q] = 1;
> > +    /* roi_map has memset with zero, just explictly set it again for explict
> understanding. */
> 
> There are some typos in this, but as the comment says, it's redundant.
> I think this and
> the next line could be removed.

thanks, will remove.

> 
> > +    roi_map->delta_q[0] = 0;
> > +    segment_id = 1;
> > +
> > +    roi = (const AVRegionOfInterest*)sd->data;
> > +    self_size = roi->self_size;
> > +    if (!self_size || sd->size % self_size != 0) {
> > +        av_log(avctx, AV_LOG_ERROR, "Invalid
> AVRegionOfInterest.self_size.\n");
> > +        return AVERROR(EINVAL);
> > +    }
> > +    nb_rois = sd->size / self_size;
> > +
> > +    /* This list must be iterated from zero because regions are
> > +     * defined in order of decreasing importance. So discard less
> > +     * important areas if they exceed the segment count.
> > +     */
> > +    for (int i = 0; i < nb_rois; i++) {
> > +        int delta_q;
> > +        int mapping_index;
> > +
> > +        roi = (const AVRegionOfInterest*)(sd->data + self_size * i);
> > +        if (roi->qoffset.den == 0) {
> > +            av_log(avctx, AV_LOG_ERROR,
> "AVRegionOfInterest.qoffset.den must not be zero.\n");
> > +            return AVERROR(EINVAL);
> > +        }
> > +
> > +        delta_q = (int)(roi->qoffset.num * 1.0f / roi->qoffset.den *
> MAX_DELTA_Q);
> > +        delta_q = av_clip(delta_q, -MAX_DELTA_Q, MAX_DELTA_Q);
> > +
> 
> Note that libvpx allows other adjustments (loop filter, block skipping, among
> others), but there may not be a way to provide this with AVRegionOfInterest.

thanks, I see, here we might use qoffset only to change the delta_q.

> 
> > [...]
> > +
> > +        if (ctx->aq_mode > 0 || ctx->cpu_used < 5 || ctx->deadline !=
> VPX_DL_REALTIME) {
> > +            if (!ctx->roi_warned) {
> > +                ctx->roi_warned = 1;
> > +                if (ctx->aq_mode > 0)
> > +                    av_log(avctx, AV_LOG_WARNING, "ROI is disabled
> when any of AQ mode is on, skipping ROI.\n");
> > +                else
> > +                    av_log(avctx, AV_LOG_WARNING, "due to libvpx's
> internal issue (see function apply_roi_map), skipping ROI.\n");
> 
> I think this could just mention that ROI is only supported with aq_mode==0,
> cpu_used >= 5 and deadline set to realtime rather than having a more specific
> statement like this one.

ok, will change it.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".