On 02/02/2020 01:33, Andriy Gelman wrote:
> On Sat, 01. Feb 22:38, Mark Thompson wrote:
>> On 19/01/2020 19:54, Andriy Gelman wrote:
>>> From: Andriy Gelman <andriy.gel...@gmail.com>
>>>
>>> Hard coded parameters for qmin and 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.gel...@gmail.com>
>>> ---
>>>  libavcodec/v4l2_m2m_enc.c | 33 +++++++++++++++++++++++++++++++--
>>>  1 file changed, 31 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavcodec/v4l2_m2m_enc.c b/libavcodec/v4l2_m2m_enc.c
>>> index 8059e3bb48f..318be0d3379 100644
>>> --- a/libavcodec/v4l2_m2m_enc.c
>>> +++ b/libavcodec/v4l2_m2m_enc.c
>>> @@ -31,10 +31,25 @@
>>>  #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
>>>  
>>> +#define CLIP_MIN_MAX(in, min_val, max_val, name, logctx)    \
>>> +    do {                                                    \
>>> +        if ((in) < (min_val)) {                             \
>>> +            av_log((logctx), AV_LOG_WARNING,                \
>>> +                   "Adjusted: " name " (%d)\n", (min_val)); \
>>> +            in = min_val;                                   \
>>> +        }                                                   \
>>> +        if ((in) > (max_val)) {                             \
>>> +            av_log((logctx), AV_LOG_WARNING,                \
>>> +                   "Adjusted: " name " (%d)\n", (max_val)); \
>>> +            (in) = (max_val);                               \
>>> +        }                                                   \
>>> +    } while (0)
>>> +
>>>  static inline void v4l2_set_timeperframe(V4L2m2mContext *s, unsigned int 
>>> num, unsigned int den)
>>>  {
>>>      struct v4l2_streamparm parm = { 0 };
>>> @@ -232,8 +247,15 @@ 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) {
>>> +        CLIP_MIN_MAX(avctx->qmin, qmin, qmax, "qmin", avctx);
>>> +        qmin = avctx->qmin;
>>> +    }
>>> +
>>> +    if (avctx->qmax >= 0) {
>>> +        CLIP_MIN_MAX(avctx->qmax, qmin, qmax, "qmax", avctx);
>>> +        qmax = avctx->qmax;
>>> +    }
>>>  
>>>      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 +371,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 +398,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", \
>>>      };
>>>
> 
>>
>> Can we avoid some of the clumsiness around clipping twice in different ways 
>> by querying the quantiser values from the encode device here?
>>
> 
> thanks, I'll investigate.
> 
>> E.g. on s5p-mfc (exynos) I can see:
>>
>>           h264_minimum_qp_value 0x00990a61 (int)    : min=0 max=51 step=1 
>> default=1 value=1
>>           h264_maximum_qp_value 0x00990a62 (int)    : min=0 max=51 step=1 
>> default=51 value=51
>>
>> which matches the expected values for 8-bit H.264, but then for VP8 there is:
>>
>>            vpx_minimum_qp_value 0x00990afb (int)    : min=0 max=11 step=1 
>> default=0 value=0
>>            vpx_maximum_qp_value 0x00990afc (int)    : min=0 max=127 step=1 
>> default=127 value=127
>>
>> which is going to pretty much entirely stop qmin from doing anything useful, 
>> and it would be nice to diagnose that.
> 
> The max for vpx_maximum_qp_value has been bothering me. The usual qp range 
> for vp8/9 is [0,63].

Ha, this is a fun subject.

Quant range for VP8 is a 7-bit value, so [0..127].  
(<https://tools.ietf.org/html/rfc6386#section-9.6>.)

Quant range for VP9/AV1 is an 8-bit value, so [0..255].  (VP9, see §6.2.9; AV1, 
see §5.9.12.)

The libvpx/libaom software encoders present an external interface which 
compresses the range to 6 bits [0..63] (I think purely so that it looks more 
like libx264), but then immediately remap to the actual codec range before 
doing anything with the result.  Other encoders may or may not have followed 
that and used the compressed range, so you're pretty much stuck with trying it 
or looking at the source code of a given encoder to find out which one they 
used.

For V4L2 I can't find any official definition of which one is being used, and 
there are multiple different encoders behind it.

Of the three drivers in mainline Linux, two (s5p-mfc and venus) advertise that 
they use the codec range:

<https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/media/platform/qcom/venus/venc_ctrls.c?h=v5.4.17#n339>
<https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c?h=v5.4.17#n650>

while the other (mtk-vcodec) doesn't support any quantiser controls at all (I 
guess it only does bitrate targets).

> I checked today with a dev on #v4l and he told me that only 6 bits are used in
> the driver so it's probably a bug. I'll look into this further. 

If the s5p-mfc driver actually uses the libvpx range rather than the codec 
range, they should probably talk to the venus people to get agreement on what 
range the controls are using.  If venus does use the codec range but the 
s5p-mfc firmware blob wants the libvpx range then they would be better off 
putting a remapping table in their driver than changing their range to not 
match.

> On a related note, I actually haven't been able to play a stream that's been
> compressed with vp8/9 on the s5p-mfc (odroid xu4 for me). Have you had any
> luck? 

I haven't seen it make a valid stream, but I've only ever done token testing 
with it to observe that.  I assume it does work on venus, since that was what 
the Linaro people were originally testing on.

- Mark
_______________________________________________
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".

Reply via email to