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

[James Zern]

Hi,

On Mon, Aug 26, 2019 at 11:30 PM Guo, Yejun <yejun.guo at intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: James Zern [mailto:jzern at google.com]
> > Sent: Tuesday, August 27, 2019 12:03 PM
> > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > Cc: Guo, Yejun <yejun.guo at intel.com>
> > Subject: Re: [FFmpeg-devel] [PATCH V5] avcodec/libvpxenc: add ROI-based
> > encoding support for VP8/VP9 support
> >
> > 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.
>
> thanks,
> do you mean bump LIBAVCODEC_VERSION_MICRO in file libavcodec/version.h? thanks.
>

Yes.

> >
> > > [...]
> > > +
> > > +            segment_mapping[mapping_index] = segment_id + 1;
> > > +            roi_map->delta_q[segment_id] = delta_q;
> > > +            segment_id++;
> > > +        }
> > > +    }
> > > +
> > > +
> >
> > This line can go.
>
> do you mean the next line av_assert0 can be removed? thanks.
>

No, I meant the extra blank line, there are 2 of them.