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

[Guangxin Xu]

Hi  Lingjie,
thanks for the comment.
We can use codec id to do special things for a specific codec. Just like
qsv_decode_init.

thanks

On Sat, Jan 16, 2021 at 3:22 PM Linjie Fu <linjie.justin.fu at gmail.com>
wrote:

> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".