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

[Xiang, Haihao]

Hi Mark

Do you plan to merge this commit any time soon?

Thanks
Haihao


> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel