[FFmpeg-devel] [PATCH] avcodec/av1dec: check avctx->hwaccel when hwaccel pix_fmt selected
Xiang, Haihao
haihao.xiang at intel.com
Tue Sep 29 05:33:25 EEST 2020
On Sat, 2020-09-26 at 13:05 -0300, James Almer wrote:
> On 9/25/2020 4:35 AM, Xiang, Haihao wrote:
> > On Fri, 2020-09-25 at 06:10 +0000, Wang, Fei W wrote:
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Wang,
> > > > Fei W
> > > > Sent: Tuesday, September 22, 2020 11:22 AM
> > > > To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: check avctx->hwaccel
> > > > when hwaccel pix_fmt selected
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > > > James Almer
> > > > > Sent: Friday, September 18, 2020 8:57 PM
> > > > > To: ffmpeg-devel at ffmpeg.org
> > > > > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: check
> > > > > avctx->hwaccel when hwaccel pix_fmt selected
> > > > >
> > > > > On 9/18/2020 2:40 AM, Wang, Fei W wrote:
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> > > > > > > Hendrik Leppkes
> > > > > > > Sent: Thursday, September 17, 2020 5:21 PM
> > > > > > > To: FFmpeg development discussions and patches
> > > > > > > <ffmpeg-devel at ffmpeg.org>
> > > > > > > Subject: Re: [FFmpeg-devel] [PATCH] avcodec/av1dec: check
> > > > > > > avctx->hwaccel when hwaccel pix_fmt selected
> > > > > > >
> > > > > > > On Thu, Sep 17, 2020 at 10:38 AM Fei Wang <fei.w.wang at intel.com>
> > > >
> > > > wrote:
> > > > > > > >
> > > > > > > > Pix fmt with hwaccel flag may not be chosen in format probing,
> > > > > > > > in
> > > > > > > > this case avctx->hwaccel will not be inited.
> > > > > > > >
> > > > > > > > Signed-off-by: Fei Wang <fei.w.wang at intel.com>
> > > > > > > > ---
> > > > > > > > libavcodec/av1dec.c | 12 ++++++++----
> > > > > > > > 1 file changed, 8 insertions(+), 4 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index
> > > > > > > > 0bb04a3e44..cdcc618013 100644
> > > > > > > > --- a/libavcodec/av1dec.c
> > > > > > > > +++ b/libavcodec/av1dec.c
> > > > > > > > @@ -251,6 +251,7 @@ static int get_pixel_format(AVCodecContext
> > > > > > > > *avctx) {
> > > > > > > > AV1DecContext *s = avctx->priv_data;
> > > > > > > > const AV1RawSequenceHeader *seq = s->raw_seq;
> > > > > > > > + const AVPixFmtDescriptor *desc;
> > > > > > > > uint8_t bit_depth;
> > > > > > > > int ret;
> > > > > > > > enum AVPixelFormat pix_fmt = AV_PIX_FMT_NONE; @@ -327,10
> > > > > > > > +328,13 @@ static int get_pixel_format(AVCodecContext *avctx)
> > > > > > > > * Since now the av1 decoder doesn't support native decode,
> > > > > > > > if
> > > > > > > > it will be
> > > > > > > > * implemented in the future, need remove this check.
> > > > > > > > */
> > > > > > > > - if (!avctx->hwaccel) {
> > > > > > > > - av_log(avctx, AV_LOG_ERROR, "Your platform doesn't
> > > > > > > > suppport"
> > > > > > > > - " hardware accelerated AV1 decoding.\n");
> > > > > > > > - return AVERROR(ENOSYS);
> > > > > > > > + desc = av_pix_fmt_desc_get(ret);
> > > > > > > > + if (desc && (desc->flags & AV_PIX_FMT_FLAG_HWACCEL)) {
> > > > > > > > + if (!avctx->hwaccel) {
> > > > > > > > + av_log(avctx, AV_LOG_ERROR, "Your platform doesn't
> > > > > > > > suppport"
> > > > > > > > + " hardware accelerated AV1 decoding.\n");
> > > > > > > > + return AVERROR(ENOSYS);
> > > > > > > > + }
> > > > > > > > }
> > > > > > > >
> > > > > > >
> > > > > > > Isn't it supposed to quit here, because we do not have software
> > > > > > > decoding?
> > > > > >
> > > > > > Since now av1 decoder allow probe, that will try to decode first
> > > > > > frame to find stream info in open_file stage. While the hwaccel will
> > > > > > not
> > > > > > be
> > > >
> > > > inited.
> > > > > > If without change here, the error log will be print out but later
> > > > > > during transcode stage, it can gives out the correct output.
> > > > >
> > > > > If you let it choose a software pix_fmt, it will think the decoder can
> > > > > actually output something, which is not true.
> > > >
> > > > It's better not let ff_thread_get_format choose a software pix fmt here,
> > > > but
> > > > for
> > > > VAAPI the avctx->hw_device_ctx hasn't been created in probing so that
> > > > hwaccel
> > > > can not be inited and hwaccel pix fmt will not be chosen(same mechanism
> > > > with
> > > > other codecs). And then next available pix fmt in input array will be
> > > > considered.
> > > >
> > > > > Probing works fine even if it fails at this point. It will have set
> > > > > enough AVCodecContext fields to allow things like remuxing, hence the
> > > > > relevant fate tests succeeding as is.
> > > >
> > > > Yes, probing allows fail here. How about to keep the original
> > > > check(!avctx-
> > > > > hwaccel), and change log context as like "Hardware context not created
> > > > > or
> > > >
> > > > your platform doesn't support hardware accelerated AV1 decoding" and
> > > > change
> > > > log level to WARNING ? It's really confused if error log printed but
> > > > decode
> > > > works
> > > > correctly.
> > >
> > > Kindly ping @James Almer, any comments to change log context and level
> > > here?
> >
> >
> > Since commit e46f34e8 was merged, I also saw the same error message when I
> > tested my QSV av1 patch (
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2020-September/270234.html) however
> > the
> > command worked well for me.
> >
> > [av1 @ 0x55b1604c42d0] Your platform doesn't suppport hardware accelerated
> > AV1
> > decoding.
> > [av1 @ 0x55b1604c42d0] Failed to get pixel format.
> > [av1 @ 0x55b1604c42d0] video_get_buffer: image parameters invalid
> > [av1 @ 0x55b1604c42d0] get_buffer() failed
> > [av1 @ 0x55b1604c42d0] thread_get_buffer() failed
> > [av1 @ 0x55b1604c42d0] Failed to allocate space for current frame.
> > [av1 @ 0x55b1604c42d0] Get current frame error
> >
> > I agree with Fei that the user will be confused with this error message.
> >
> > Thanks
> > Haihao
>
> I guess we could change it to warning, but every other printed message
> will be an error and there's not much else we can do.
>
> I sent a patchset that prevents calling get_buffer() when no pix_fmt is
> set, so the errors it will print from then on will be less scary.
What's the subject of your patchset? I'd like to give a try with your patchset.
Thanks
Haihao
>
> >
> >
> > >
> > > Fei
> > > Thanks
> > >
> > > >
> > > >
> > > > > _______________________________________________
> > > > > 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".
> > > >
> > > > _______________________________________________
> > > > 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".
> > >
> > > _______________________________________________
> > > 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".
>
> _______________________________________________
> 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".
More information about the ffmpeg-devel
mailing list