> -----Original Message----- > From: James Zern [mailto:jz...@google.com] > Sent: Thursday, August 29, 2019 2:14 PM > To: Guo, Yejun <yejun....@intel.com> > Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH V5] avcodec/libvpxenc: add ROI-based > encoding support for VP8/VP9 support > > On Wed, Aug 28, 2019 at 10:20 PM Guo, Yejun <yejun....@intel.com> wrote: > > > > > > > > > -----Original Message----- > > > From: James Zern [mailto:jz...@google.com] > > > Sent: Thursday, August 29, 2019 12:39 PM > > > To: Guo, Yejun <yejun....@intel.com> > > > Cc: FFmpeg development discussions and patches > <ffmpeg-devel@ffmpeg.org> > > > Subject: Re: [FFmpeg-devel] [PATCH V5] avcodec/libvpxenc: add ROI-based > > > encoding support for VP8/VP9 support > > > > > > Hi, > > > > > > On Mon, Aug 26, 2019 at 11:30 PM Guo, Yejun <yejun....@intel.com> > wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: James Zern [mailto:jz...@google.com] > > > > > Sent: Tuesday, August 27, 2019 12:03 PM > > > > > To: FFmpeg development discussions and patches > > > <ffmpeg-devel@ffmpeg.org> > > > > > Cc: Guo, Yejun <yejun....@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....@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....@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. > > > > My intention for the two blank lines was to divide the two logical parts > > more > clear. Anyway, I'll follow to remove. > > > > If there's a reason to divide them then a separate function or comment > for the next section could work. Extra blank lines are likely to be > cleaned up by tools or other developers. > > > and I've send out v7 for this patch this morning, do you have any other > comment? thanks. So I can combine all the comments in v8 patch. > > > > I think the last set was all I had, though I could give it more testing.
got it, thanks. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".