diff mbox series

[FFmpeg-devel,v1] avcodec/av1dec: check if hwaccel is specificed

Message ID 20210827115412.9663-1-fei.w.wang@intel.com
State New
Headers show
Series [FFmpeg-devel,v1] avcodec/av1dec: check if hwaccel is specificed
Related show

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Wang, Fei W Aug. 27, 2021, 11:54 a.m. UTC
Since av1 decoder is only available for hw acceleration. This will
gives out more accurate information if this decoder used but doesn't
specificed a hwaccel.

For example:
ffmpeg -c:v av1 -i INPUT OUTPUT

Signed-off-by: Fei Wang <fei.w.wang@intel.com>
---
1. This is improvement of patch for:
https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4660

 libavcodec/av1dec.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Soft Works Aug. 27, 2021, 4:48 p.m. UTC | #1
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Fei
> Wang
> Sent: Friday, 27 August 2021 13:54
> To: ffmpeg-devel@ffmpeg.org
> Cc: Fei Wang <fei.w.wang@intel.com>
> Subject: [FFmpeg-devel] [PATCH v1] avcodec/av1dec: check if hwaccel
> is specificed
> 
> Since av1 decoder is only available for hw acceleration. This will
> gives out more accurate information if this decoder used but doesn't
> specificed a hwaccel.
> 
> For example:
> ffmpeg -c:v av1 -i INPUT OUTPUT
> 
> Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> ---
> 1. This is improvement of patch for:
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4660
> 
>  libavcodec/av1dec.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index a69808f7b6..58a9deeb4e 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -452,15 +452,22 @@ static int get_pixel_format(AVCodecContext
> *avctx)
>      *fmtp++ = s->pix_fmt;
>      *fmtp = AV_PIX_FMT_NONE;
> 
> -    ret = ff_thread_get_format(avctx, pix_fmts);
> -    if (ret < 0)
> -        return ret;
> -
>      /**
> -     * check if the HW accel is inited correctly. If not, return un-
> implemented.
> +     * check if the HW accel is specificed. If not, return un-
> implemented.
>       * Since now the av1 decoder doesn't support native decode, if
> it will be
>       * implemented in the future, need remove this check.
>       */
> +    if (!avctx->hw_device_ctx) {
> +        av_log(avctx, AV_LOG_ERROR, "The AV1 decoder requires a hw
> acceleration"
> +	       " to be specified.\n");
> +        return AVERROR(ENOSYS);
> +    }
> +
> +    ret = ff_thread_get_format(avctx, pix_fmts);
> +    if (ret < 0)
> +        return ret;
> +
> +    /** check if the HW accel is inited correctly by the specificed
> setting */
>      if (!avctx->hwaccel) {
>          av_log(avctx, AV_LOG_ERROR, "Your platform doesn't suppport"
>                 " hardware accelerated AV1 decoding.\n");
> --

LGTM.
Hendrik Leppkes Aug. 27, 2021, 5:09 p.m. UTC | #2
On Fri, Aug 27, 2021 at 1:54 PM Fei Wang <fei.w.wang@intel.com> wrote:
>
> Since av1 decoder is only available for hw acceleration. This will
> gives out more accurate information if this decoder used but doesn't
> specificed a hwaccel.
>
> For example:
> ffmpeg -c:v av1 -i INPUT OUTPUT
>
> Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> ---
> 1. This is improvement of patch for:
> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4660
>
>  libavcodec/av1dec.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index a69808f7b6..58a9deeb4e 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -452,15 +452,22 @@ static int get_pixel_format(AVCodecContext *avctx)
>      *fmtp++ = s->pix_fmt;
>      *fmtp = AV_PIX_FMT_NONE;
>
> -    ret = ff_thread_get_format(avctx, pix_fmts);
> -    if (ret < 0)
> -        return ret;
> -
>      /**
> -     * check if the HW accel is inited correctly. If not, return un-implemented.
> +     * check if the HW accel is specificed. If not, return un-implemented.
>       * Since now the av1 decoder doesn't support native decode, if it will be
>       * implemented in the future, need remove this check.
>       */
> +    if (!avctx->hw_device_ctx) {
> +        av_log(avctx, AV_LOG_ERROR, "The AV1 decoder requires a hw acceleration"
> +              " to be specified.\n");
> +        return AVERROR(ENOSYS);
> +    }
> +

Not every hwaccel uses a hw_device_ctx. Its still perfectly valid and
supported to use the old setup which overrides get_format/get_buffer2
and sets avctx->hwaccel_context directly.
You will not know if this is the case before get_format was called, so
you cannot pre-detect it.

Or in other words, this patch breaks my application, so its not a
valid solution.

- Hendrik
Wang, Fei W Aug. 30, 2021, 1:55 a.m. UTC | #3
On Fri, 2021-08-27 at 19:09 +0200, Hendrik Leppkes wrote:
> On Fri, Aug 27, 2021 at 1:54 PM Fei Wang <fei.w.wang@intel.com>
> wrote:
> > 
> > Since av1 decoder is only available for hw acceleration. This will
> > gives out more accurate information if this decoder used but
> > doesn't
> > specificed a hwaccel.
> > 
> > For example:
> > ffmpeg -c:v av1 -i INPUT OUTPUT
> > 
> > Signed-off-by: Fei Wang <fei.w.wang@intel.com>
> > ---
> > 1. This is improvement of patch for:
> > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=4660
> > 
> >  libavcodec/av1dec.c | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> > index a69808f7b6..58a9deeb4e 100644
> > --- a/libavcodec/av1dec.c
> > +++ b/libavcodec/av1dec.c
> > @@ -452,15 +452,22 @@ static int get_pixel_format(AVCodecContext
> > *avctx)
> >      *fmtp++ = s->pix_fmt;
> >      *fmtp = AV_PIX_FMT_NONE;
> > 
> > -    ret = ff_thread_get_format(avctx, pix_fmts);
> > -    if (ret < 0)
> > -        return ret;
> > -
> >      /**
> > -     * check if the HW accel is inited correctly. If not, return
> > un-implemented.
> > +     * check if the HW accel is specificed. If not, return un-
> > implemented.
> >       * Since now the av1 decoder doesn't support native decode, if
> > it will be
> >       * implemented in the future, need remove this check.
> >       */
> > +    if (!avctx->hw_device_ctx) {
> > +        av_log(avctx, AV_LOG_ERROR, "The AV1 decoder requires a hw
> > acceleration"
> > +              " to be specified.\n");
> > +        return AVERROR(ENOSYS);
> > +    }
> > +
> 
> Not every hwaccel uses a hw_device_ctx. Its still perfectly valid and
> supported to use the old setup which overrides get_format/get_buffer2
> and sets avctx->hwaccel_context directly.
> You will not know if this is the case before get_format was called,
> so
> you cannot pre-detect it.

If so, the only way to fix is keep previous check with avctx->hwaccel
but change its log context like "The AV1 decoder requires a hw acceleration to be specified or the specified hw doesn't support AV1 decoding."

Btw, just for curious, which hwaccel are you using that doesn't set up
the hw_device_ctx?

> 
> Or in other words, this patch breaks my application, so its not a
> valid solution.
> 
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Hendrik Leppkes Aug. 30, 2021, 8:39 a.m. UTC | #4
On Mon, Aug 30, 2021 at 3:56 AM Wang, Fei W <fei.w.wang@intel.com> wrote:
>
> If so, the only way to fix is keep previous check with avctx->hwaccel
> but change its log context like "The AV1 decoder requires a hw acceleration to be specified or the specified hw doesn't support AV1 decoding."
>

That sounds right, you can change the message of course, but I don't
think you can necessarily change the logic.

> Btw, just for curious, which hwaccel are you using that doesn't set up
> the hw_device_ctx?
>

I'm using DXVA2 that way, but also VA-API, VDPAU and VideoToolbox are
all setup to use the old hwaccel initialization.

- Hendrik
diff mbox series

Patch

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index a69808f7b6..58a9deeb4e 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -452,15 +452,22 @@  static int get_pixel_format(AVCodecContext *avctx)
     *fmtp++ = s->pix_fmt;
     *fmtp = AV_PIX_FMT_NONE;
 
-    ret = ff_thread_get_format(avctx, pix_fmts);
-    if (ret < 0)
-        return ret;
-
     /**
-     * check if the HW accel is inited correctly. If not, return un-implemented.
+     * check if the HW accel is specificed. If not, return un-implemented.
      * Since now the av1 decoder doesn't support native decode, if it will be
      * implemented in the future, need remove this check.
      */
+    if (!avctx->hw_device_ctx) {
+        av_log(avctx, AV_LOG_ERROR, "The AV1 decoder requires a hw acceleration"
+	       " to be specified.\n");
+        return AVERROR(ENOSYS);
+    }
+
+    ret = ff_thread_get_format(avctx, pix_fmts);
+    if (ret < 0)
+        return ret;
+
+    /** check if the HW accel is inited correctly by the specificed setting */
     if (!avctx->hwaccel) {
         av_log(avctx, AV_LOG_ERROR, "Your platform doesn't suppport"
                " hardware accelerated AV1 decoding.\n");