| 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 | expand |
| Context | Check | Description |
|---|---|---|
| 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 |
> -----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.
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
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".
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 --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");
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(-)