[FFmpeg-devel] avcodec/v4l2_m2m_enc: Support changing qmin/qmax

From: Andriy Gelman <andriy.gelman@gmail.com>

Hard coded parameters for qmin/qmax are currently used to initialize
v4l2_m2m device. This commit uses values from avctx->{qmin,qmax} if they
are set.

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libavcodec/v4l2_m2m_enc.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

On 19-01-2020 01:41 pm, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
>
> Hard coded parameters for qmin/qmax are currently used to initialize
> v4l2_m2m device. This commit uses values from avctx->{qmin,qmax} if they
> are set.
>
> Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> ---
>   libavcodec/v4l2_m2m_enc.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
> index 8059e3bb48f..b61d48316ee 100644
> --- a/libavcodec/v4l2_m2m_enc.c
> +++ b/libavcodec/v4l2_m2m_enc.c
> @@ -31,6 +31,7 @@
>   #include "v4l2_context.h"
>   #include "v4l2_m2m.h"
>   #include "v4l2_fmt.h"
> +#include "internal.h"
>   
>   #define MPEG_CID(x) V4L2_CID_MPEG_VIDEO_##x
>   #define MPEG_VIDEO(x) V4L2_MPEG_VIDEO_##x
> @@ -232,8 +233,11 @@ static int v4l2_prepare_encoder(V4L2m2mContext *s)
>           return 0;
>       }
>   
> -    if (qmin != avctx->qmin || qmax != avctx->qmax)
> -        av_log(avctx, AV_LOG_WARNING, "Encoder adjusted: qmin (%d), qmax (%d)\n", qmin, qmax);
> +    if (avctx->qmin >= 0)
> +        qmin = avctx->qmin;
> +
> +    if (avctx->qmax >= 0)
> +        qmax = avctx->qmax;

Each encoder has its own range and thus scale. You should clip as per 
the range and document the ranges per codec.
One possible complication is that avctx qmin can go upto 69 but VP8/9's 
range extends beyond that. If qmin > 69 is
legal for VP8/9, then the user won't be able to set it. We can either 
extend the range in avctx or add a private option here.

>   
>       v4l2_set_ext_ctrl(s, qmin_cid, qmin, "minimum video quantizer scale");
>       v4l2_set_ext_ctrl(s, qmax_cid, qmax, "maximum video quantizer scale");
> @@ -349,6 +353,12 @@ static const AVOption options[] = {
>       { NULL },
>   };
>   
> +static const AVCodecDefault v4l2_m2m_defaults[] = {
> +    { "qmin", "-1" },
> +    { "qmax", "-1" },
> +    { NULL },
> +};
> +
>   #define M2MENC_CLASS(NAME) \
>       static const AVClass v4l2_m2m_ ## NAME ## _enc_class = { \
>           .class_name = #NAME "_v4l2m2m_encoder", \
> @@ -370,6 +380,7 @@ static const AVOption options[] = {
>           .send_frame     = v4l2_send_frame, \
>           .receive_packet = v4l2_receive_packet, \
>           .close          = v4l2_encode_close, \
> +        .defaults       = v4l2_m2m_defaults, \
>           .capabilities   = AV_CODEC_CAP_HARDWARE | AV_CODEC_CAP_DELAY, \
>           .wrapper_name   = "v4l2m2m", \
>       };

On a related note, I see that an  encoder is defined for hevc and not 
for VP9. However, prepare_encoder has a case for vp9 and not for hevc. 
What am I missing here?

Gyan

On Sun, 19. Jan 16:44, Gyan wrote:
> 
> 
> On 19-01-2020 01:41 pm, Andriy Gelman wrote:
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> > 
> > Hard coded parameters for qmin/qmax are currently used to initialize
> > v4l2_m2m device. This commit uses values from avctx->{qmin,qmax} if they
> > are set.
> > 
> > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > ---
> >   libavcodec/v4l2_m2m_enc.c | 15 +++++++++++++--
> >   1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
> > index 8059e3bb48f..b61d48316ee 100644
> > --- a/libavcodec/v4l2_m2m_enc.c
> > +++ b/libavcodec/v4l2_m2m_enc.c
> > @@ -31,6 +31,7 @@
> >   #include "v4l2_context.h"
> >   #include "v4l2_m2m.h"
> >   #include "v4l2_fmt.h"
> > +#include "internal.h"
> >   #define MPEG_CID(x) V4L2_CID_MPEG_VIDEO_##x
> >   #define MPEG_VIDEO(x) V4L2_MPEG_VIDEO_##x
> > @@ -232,8 +233,11 @@ static int v4l2_prepare_encoder(V4L2m2mContext *s)
> >           return 0;
> >       }
> > -    if (qmin != avctx->qmin || qmax != avctx->qmax)
> > -        av_log(avctx, AV_LOG_WARNING, "Encoder adjusted: qmin (%d), qmax (%d)\n", qmin, qmax);
> > +    if (avctx->qmin >= 0)
> > +        qmin = avctx->qmin;
> > +
> > +    if (avctx->qmax >= 0)
> > +        qmax = avctx->qmax;

> 
> Each encoder has its own range and thus scale. You should clip as per the
> range and document the ranges per codec.

I went back and forth about clipping. Ended up not adding it after looking at
examples from libx264/5.c where they except the values as is. 
I'll add clipping in the next version. 

Btw, I have a feeling that these parameters may not be well supported in
v4l2_m2m. At least when I tested on the Raspi4 and Odroid XU4: On the Raspi4,
the ioctl call was not accepted (with any value of qmin/qmax), on the Odroid XU4
the ioctl call worked but didn't have any effect on the video. 
Of course, there may other devices supporting this feature.

> One possible complication is that avctx qmin can go upto 69 but VP8/9's
> range extends beyond that. If qmin > 69 is
> legal for VP8/9, then the user won't be able to set it. We can either extend
> the range in avctx or add a private option here.
> 

I'm leaning towards modifying the global range as this could also help libvpx.
I'll run some tests.

> >       v4l2_set_ext_ctrl(s, qmin_cid, qmin, "minimum video quantizer scale");
> >       v4l2_set_ext_ctrl(s, qmax_cid, qmax, "maximum video quantizer scale");
> > @@ -349,6 +353,12 @@ static const AVOption options[] = {
> >       { NULL },
> >   };
> > +static const AVCodecDefault v4l2_m2m_defaults[] = {
> > +    { "qmin", "-1" },
> > +    { "qmax", "-1" },
> > +    { NULL },
> > +};
> > +
> >   #define M2MENC_CLASS(NAME) \
> >       static const AVClass v4l2_m2m_ ## NAME ## _enc_class = { \
> >           .class_name = #NAME "_v4l2m2m_encoder", \
> > @@ -370,6 +380,7 @@ static const AVOption options[] = {
> >           .send_frame     = v4l2_send_frame, \
> >           .receive_packet = v4l2_receive_packet, \
> >           .close          = v4l2_encode_close, \
> > +        .defaults       = v4l2_m2m_defaults, \
> >           .capabilities   = AV_CODEC_CAP_HARDWARE | AV_CODEC_CAP_DELAY, \
> >           .wrapper_name   = "v4l2m2m", \
> >       };

> 
> On a related note, I see that an  encoder is defined for hevc and not for
> VP9. However, prepare_encoder has a case for vp9 and not for hevc. What am I
> missing here?

Yeah, good catch again. For vp9, I'll do some research to see if there are
devices supporting it. We should consider adding it. 

For hevc encoder, I tried to fix missing qmin/qmax case (also missing profile
setup) in the past, but couldn't find a "workable" v4l2_m2m device to test.

One option is Nvidia's Jetson Nano, but there were too many
issues when I tried to implement/test with ffmpeg. It required an extra header
with incompatible license, ioctl calls with libv4l2, broken timestamps, and it
had some weird race conditions.. 
FYI, it looks that Nvidia recently added support for interfacing directly to
NVENC [1], so there is no longer a need to use v4l2_m2m interface (although I haven't
tested NVENC on the Nano yet)
[1] https://developer.nvidia.com/embedded/jetpack

Thanks,