[FFmpeg-devel] [PATCH, RFC, v2 3/3] lavc/libvpxenc: add dynamic resolution encode support for libvpx

[Fu, Linjie]

> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of James Zern
> Sent: Wednesday, August 14, 2019 07:37
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH, RFC, v2 3/3] lavc/libvpxenc: add
> dynamic resolution encode support for libvpx
> 
> Hi,
> 
> On Tue, Aug 13, 2019 at 2:30 AM Linjie Fu <linjie.fu at intel.com> wrote:
> >
> > According to spec, libvpx should support dynamic resolution changes.
> >
> > Add dynamic resolution encoding support in libvpx.
> >
> > Only single pass mode with no look ahead is supported for variable
> > resolution encoding without initialization.
> >
> 
> Do you mean /reinitialization/?

Yes, should be reinitialization exactly.

> 
> > cmdline:
> > ffmpeg -noautoscale -y -i ./reinit-large_420_8-to-small_420_8.h264
> >      -pix_fmt yuv420p -c:v libvpx-vp9 lena.ivf
> >
> 
> Do you have a reference command line for creating the source content?

This clips is accessible at
http://fate-suite.ffmpeg.org/h264/reinit-large_420_8-to-small_420_8.h264

However, mentioned issue could not be reproduced with above clips actually.

> 
> > Filed an issue in
> https://bugs.chromium.org/p/webm/issues/detail?id=1642
> > to fix some memory problem.
> >
> 
> It may be worth getting that bug resolved before landing this change
> if existing library versions are buggy.

Sure, this patch should be hold until bug is fixed in libvpx. 

> > Signed-off-by: Linjie Fu <linjie.fu at intel.com>
> > ---
> >  libavcodec/libvpxenc.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > index feb52ea..3d2295d 100644
> > --- a/libavcodec/libvpxenc.c
> > +++ b/libavcodec/libvpxenc.c
> > @@ -1067,6 +1067,28 @@ static int vpx_encode(AVCodecContext *avctx,
> AVPacket *pkt,
> >      int res, coded_size;
> >      vpx_enc_frame_flags_t flags = 0;
> >
> > +    if (frame && (avctx->width != frame->width ||
> > +                  avctx->height != frame->height)) {
> > +        avctx->width  = frame->width;
> > +        avctx->height = frame->height;
> > +
> > +        struct vpx_codec_enc_cfg new_cfg = { 0 };
> > +        memcpy(&new_cfg, ctx->encoder.config.enc,
> > +                            sizeof(struct vpx_codec_enc_cfg));
> > +
> > +        new_cfg.g_w = frame->width;
> > +        new_cfg.g_h = frame->height;
> > +        if (new_cfg.g_lag_in_frames > 1 ||
> > +            new_cfg.g_pass != VPX_RC_ONE_PASS) {
> > +            av_log(avctx, AV_LOG_WARNING, "Only single pass mode "
> > +                   "with no look ahead is supported for variable "
> > +                   "resolution encoding without initialization.\n");
> 
> Would it be better to warn and reinitialize as in your earlier patch
> rather than changing the settings from the user?

Warn and reinitialize seems better for me.

Will resend after all concerns are addressed.
Thanks.

- linjie