[FFmpeg-devel] [PATCH] lavc/qsvdec: allow qsv decoders to use initialized device

[Rogozhkin, Dmitry V]

On Sun, 2020-09-06 at 15:25 +0100, Mark Thompson wrote:
> On 04/09/2020 19:24, Rogozhkin, Dmitry V wrote:
> > On Thu, 2020-09-03 at 01:02 +0000, Rogozhkin, Dmitry V wrote:
> > > > 
> > > > (If you do implement it then you can delete all of the ad-hoc
> > > > treatment in ffmpeg, like has been done for the other hardware
> > > > codecs.)
> > > 
> > > I like deleting code:). Ok, this sounds good. Let's try to
> > > understand
> > > what might be missing in the current implementation since I
> > > honestly
> > > don't see any gaps - it just works.
> > 
> > @Mark. We did internal review and believe that DEVICE_CTX path is
> > actually ready to be used and just needs to be activated. Can you,
> > please, let me know how you would like to proceed:
> > 1. We can either consider review and apply the fix first (this
> > patch)
> > then deal with ad_hoc in non-related patch series
> > 2. Or we can go with the bigger patch series right away and address
> > both device_ctx + ad_hoc
> > 
> > 2nd variant might require longer time to verify and review which
> > would
> > hold the fix. What are your thoughts?
> 
> I'm not sure what you have tested, because it definitely doesn't
> work.

I was testing transcode and indeed I overlooked that it falls into
using system memory instead of HW frames. My fault.

> 
> If you return the hardware surface format from get_format() with
> METHOD_HW_DEVICE_CTX then it just ignores you and gives you software
> frames anyway, because it only supports that case with
> METHOD_HW_FRAMES_CTX.

Can you, please, guide me towards what should be implemented/fixed in
qsv path? I afraid that implementing this fully on my own is beyond of
my current knowledge of qsv path in ffmpeg. Some guidance would be
appreciated. Maybe there is implementation for some other accel type I
can refer to?

> 
> For example, with below patch to test it in the hw_decode example:
> 
> $ doc/examples/hw_decode qsv test.264 /dev/null
> Assertion frame->format == AV_PIX_FMT_QSV failed at
> src/doc/examples/hw_decode.c:108
> Aborted
> 
> (It incorrectly returned an NV12 frame.)
> 
> - Mark
> 
> 
> diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c
> index 71be6e6709..45f401a179 100644
> --- a/doc/examples/hw_decode.c
> +++ b/doc/examples/hw_decode.c
> @@ -105,6 +105,8 @@ static int decode_write(AVCodecContext *avctx,
> AVPacket *packet)
>               goto fail;
>           }
> 
> +        av_assert0(frame->format == AV_PIX_FMT_QSV);
> +
>           if (frame->format == hw_pix_fmt) {
>               /* retrieve data from GPU to CPU */
>               if ((ret = av_hwframe_transfer_data(sw_frame, frame,
> 0)) < 0) {
> @@ -191,6 +193,8 @@ int main(int argc, char *argv[])
>       }
>       video_stream = ret;
> 
> +    decoder = avcodec_find_decoder_by_name("h264_qsv");
> +
>       for (i = 0;; i++) {
>           const AVCodecHWConfig *config =
> avcodec_get_hw_config(decoder, i);
>           if (!config) {
> diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
> index fc25dc73e5..f2fac17354 100644
> --- a/libavcodec/qsvdec.c
> +++ b/libavcodec/qsvdec.c
> @@ -47,7 +47,8 @@ const AVCodecHWConfigInternal *ff_qsv_hw_configs[]
> = {
>       &(const AVCodecHWConfigInternal) {
>           .public = {
>               .pix_fmt     = AV_PIX_FMT_QSV,
> -            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
> +            .methods     = AV_CODEC_HW_CONFIG_METHOD_HW_DEVICE_CTX |
> +                           AV_CODEC_HW_CONFIG_METHOD_HW_FRAMES_CTX |
>                              AV_CODEC_HW_CONFIG_METHOD_AD_HOC,
>               .device_type = AV_HWDEVICE_TYPE_QSV,
>           },
> _______________________________________________
> 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".