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

[Mark Thompson]

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.

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.

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,
          },