[FFmpeg-devel] [PATCH 11/26] vaapi_encode: Add common options between all encoders

[Mark Thompson]

On 25/04/18 02:21, Zhao Jun wrote:
> 2018-04-22 23:29 GMT+08:00 Mark Thompson <sw at jkqxz.net>:
>> The only common option here is low_power - it was previously supported
>> for H.264 only, that specific option is removed.
>> ---
>>  doc/encoders.texi              | 14 ++++++++++++--
>>  libavcodec/vaapi_encode.h      |  9 +++++++++
>>  libavcodec/vaapi_encode_h264.c |  8 ++------
>>  libavcodec/vaapi_encode_h265.c |  2 ++
>>  libavcodec/vaapi_encode_vp8.c  |  1 +
>>  libavcodec/vaapi_encode_vp9.c  |  1 +
>>  6 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/doc/encoders.texi b/doc/encoders.texi
>> index 16be6359b3..62a1509a96 100644
>> --- a/doc/encoders.texi
>> +++ b/doc/encoders.texi
>> @@ -2599,6 +2599,18 @@ Size / quality tradeoff: higher values are smaller / worse quality.
>>  @option{b_qoffset} / @option{b_quant_offset}
>>  @end itemize
>>
>> +All encoders support the following options:
>> + at itemize
>> + at item
>> + at option{low_power}
>> +
>> +Some drivers/platforms offer a second encoder for some codecs intended to use
>> +less power than the default encoder; setting this option will attempt to use
>> +that encoder.  Note that it may support a reduced feature set, so some other
>> +options may not be available in this mode.
>> + at end itemize
>> +
>> +Each encoder also has its own specific options:
>>  @table @option
>>
>>  @item h264_vaapi
>> @@ -2606,8 +2618,6 @@ Size / quality tradeoff: higher values are smaller / worse quality.
>>  @option{level} sets the value of @emph{level_idc}.
>>
>>  @table @option
>> - at item low_power
>> -Use low-power encoding mode.
>>  @item coder
>>  Set entropy encoder (default is @emph{cabac}).  Possible values:
>>
>> diff --git a/libavcodec/vaapi_encode.h b/libavcodec/vaapi_encode.h
>> index 2b909d4d6b..9e0826b30e 100644
>> --- a/libavcodec/vaapi_encode.h
>> +++ b/libavcodec/vaapi_encode.h
>> @@ -300,4 +300,13 @@ int ff_vaapi_encode2(AVCodecContext *avctx, AVPacket *pkt,
>>  int ff_vaapi_encode_init(AVCodecContext *avctx);
>>  int ff_vaapi_encode_close(AVCodecContext *avctx);
>>
>> +
>> +#define VAAPI_ENCODE_COMMON_OPTIONS \
>> +    { "low_power", \
>> +      "Use low-power encoding mode (only available on some platforms; " \
>> +      "may not support all encoding features)", \
>> +      OFFSET(common.low_power), AV_OPT_TYPE_BOOL, \
>> +      { .i64 = 0 }, 0, 1, FLAGS }
>> +
>> +
>>  #endif /* AVCODEC_VAAPI_ENCODE_H */
>> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
>> index 93dc51eb40..49dc5229b5 100644
>> --- a/libavcodec/vaapi_encode_h264.c
>> +++ b/libavcodec/vaapi_encode_h264.c
>> @@ -52,7 +52,6 @@ typedef struct VAAPIEncodeH264Context {
>>      // User options.
>>      int qp;
>>      int quality;
>> -    int low_power;
> 
> The patchset other part is Ok for me, but I don't think putting
> low_power as common
> option is good idea, as I know, low_power mode only supported by few
> codec, maybe
> only support by AVC/HEVC/VP9 encoder.

That is just what is supported on the i965 driver on Cannonlake at the moment, right?  There isn't anything stopping it being supported for other codecs in any driver, and since we query the entrypoints at runtime the behaviour if it isn't there is just the same as if run on the i965 driver with an Intel chip not supporting VDENC for H.264.

- Mark