[FFmpeg-devel] [PATCH v2 6/6] avcodec/qsvdec: refact, remove duplicate code for plugin loading

[Linjie Fu]

Guangxin:

On Tue, Jan 5, 2021 at 10:44 AM Xu Guangxin <guangxin.xu at intel.com> wrote:
>
> ---
>  libavcodec/qsvdec.c | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
> index 3ca16dafae..d10f90a0db 100644
> --- a/libavcodec/qsvdec.c
> +++ b/libavcodec/qsvdec.c
> @@ -682,21 +682,12 @@ static av_cold int qsv_decode_init(AVCodecContext *avctx)
>  {
>      QSVDecContext *s = avctx->priv_data;
>      int ret;
> +    const char *uid = NULL;
>
>      if (avctx->codec_id == AV_CODEC_ID_VP8) {
> -        static const char *uid_vp8dec_hw = "f622394d8d87452f878c51f2fc9b4131";
> -
> -        av_freep(&s->qsv.load_plugins);
> -        s->qsv.load_plugins = av_strdup(uid_vp8dec_hw);
> -        if (!s->qsv.load_plugins)
> -            return AVERROR(ENOMEM);
> +        uid = "f622394d8d87452f878c51f2fc9b4131";
>      } else if (avctx->codec_id == AV_CODEC_ID_VP9) {
> -        static const char *uid_vp9dec_hw = "a922394d8d87452f878c51f2fc9b4131";
> -
> -        av_freep(&s->qsv.load_plugins);
> -        s->qsv.load_plugins = av_strdup(uid_vp9dec_hw);
> -        if (!s->qsv.load_plugins)
> -            return AVERROR(ENOMEM);
> +        uid = "a922394d8d87452f878c51f2fc9b4131";
>      }
>      else if (avctx->codec_id == AV_CODEC_ID_HEVC && s->load_plugin != LOAD_PLUGIN_NONE) {
>          static const char * const uid_hevcdec_sw = "15dd936825ad475ea34e35f3f54217a6";
> @@ -707,16 +698,18 @@ static av_cold int qsv_decode_init(AVCodecContext *avctx)
>                     "load_plugins is not empty, but load_plugin is not set to 'none'."
>                     "The load_plugin value will be ignored.\n");
>          } else {
> -            av_freep(&s->qsv.load_plugins);
> -
>              if (s->load_plugin == LOAD_PLUGIN_HEVC_SW)
> -                s->qsv.load_plugins = av_strdup(uid_hevcdec_sw);
> +                uid = uid_hevcdec_sw;
>              else
> -                s->qsv.load_plugins = av_strdup(uid_hevcdec_hw);
> -            if (!s->qsv.load_plugins)
> -                return AVERROR(ENOMEM);
> +                uid = uid_hevcdec_hw;
>          }
>      }
> +    if (uid) {
> +        av_freep(&s->qsv.load_plugins);
> +        s->qsv.load_plugins = av_strdup(uid);
> +        if (!s->qsv.load_plugins)
> +            return AVERROR(ENOMEM);
> +    }
>
>      s->qsv.orig_pix_fmt = AV_PIX_FMT_NV12;
>      s->packet_fifo = av_fifo_alloc(sizeof(AVPacket));
> --
Merging the AVCodec descriptions for all qsv decoders makes the code
cleaner, since
the majority of them are identical. And all checks passed in [1].

One concern is it may be less convenient or more tricky to modify in
the future, if a
specific decoder changes and differs from the rest.

Anyway, lgtm at least for now, and prefer to apply if no more
comments/objections/concerns.

[1] https://github.com/intel-media-ci/ffmpeg/pull/326

- linjie