[FFmpeg-devel] [PATCH] avcodec/av1dec: check avctx->hwaccel when hwaccel pix_fmt selected

[James Almer]

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.

> 
> 
>>
>> 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".