diff mbox

[FFmpeg-devel,2/2] ffmpeg: Set default output format for cuvid hwaccel

Message ID 20170618232730.14614-3-philipl@overt.org
State New
Headers show

Commit Message

Philip Langdale June 18, 2017, 11:27 p.m. UTC
The cuvid hwaccel is weird because it's not a real hwaccel. This
means that requesting the hwaccel by itself does nothing as the
right decoder and encoder have to be request to take advantage of
it.

On the other hand, requesting the cuvid decoder or nvenc encoder
will always be hardware accelerated because that's inherently
what they are.

The end result is that '-hwaccel cuvid' really ends up being an
option that requests the use of a shared context that allows full
hardware transcoding without copying frames back-and-forth to/from
system memory.

This differs from 'real' hwaccels, which plug into the existing
decoders (and encoders?). In this case, the default format is
controlled by the decoder/encoder and not the presence of the hwaccel.
So, for example, with vaapi, the hardware will decode the video but
it will be automatically converted to a software format in system
memory unless the output format is explicitly set to the native
hardware one.

Switching cuvid to be a generic hwaccel brings this later behaviour
to cuvid; specifying the hwaccel by itself without an output format
does exactly nothing - the decoder and encoder continue to define
their own contexts and frames pass through system memory.

More importantly, this changes the behaviour of command lines that
used to do full transcoding - a new argument must be added to
specify the output format.

To avoid breaking this compatibility, one possible solution is to
allow an hwaccel to indicate that its default output format is its
native hardware format, and that is what is implemented in this
change.

We believe that the qsv hwaccel also has the same pre-generic
behaviour and could also be a candidate for this.

Signed-off-by: Philip Langdale <philipl@overt.org>
---
 ffmpeg.h     |  1 +
 ffmpeg_opt.c | 19 +++++++++++--------
 2 files changed, 12 insertions(+), 8 deletions(-)

Comments

wm4 June 19, 2017, 8:40 a.m. UTC | #1
On Sun, 18 Jun 2017 16:27:30 -0700
Philip Langdale <philipl@overt.org> wrote:

> The cuvid hwaccel is weird because it's not a real hwaccel. This
> means that requesting the hwaccel by itself does nothing as the
> right decoder and encoder have to be request to take advantage of
> it.
> 
> On the other hand, requesting the cuvid decoder or nvenc encoder
> will always be hardware accelerated because that's inherently
> what they are.
> 
> The end result is that '-hwaccel cuvid' really ends up being an
> option that requests the use of a shared context that allows full
> hardware transcoding without copying frames back-and-forth to/from
> system memory.
> 
> This differs from 'real' hwaccels, which plug into the existing
> decoders (and encoders?). In this case, the default format is
> controlled by the decoder/encoder and not the presence of the hwaccel.
> So, for example, with vaapi, the hardware will decode the video but
> it will be automatically converted to a software format in system
> memory unless the output format is explicitly set to the native
> hardware one.
> 
> Switching cuvid to be a generic hwaccel brings this later behaviour
> to cuvid; specifying the hwaccel by itself without an output format
> does exactly nothing - the decoder and encoder continue to define
> their own contexts and frames pass through system memory.
> 
> More importantly, this changes the behaviour of command lines that
> used to do full transcoding - a new argument must be added to
> specify the output format.
> 
> To avoid breaking this compatibility, one possible solution is to
> allow an hwaccel to indicate that its default output format is its
> native hardware format, and that is what is implemented in this
> change.
> 
> We believe that the qsv hwaccel also has the same pre-generic
> behaviour and could also be a candidate for this.
> 
> Signed-off-by: Philip Langdale <philipl@overt.org>

I still think all hwaccels should show the same behavior. It's just a
confusing subtle difference, and might even think that there's
something "better" about cuda/qsv over dxva.

>      { "vdpau", hwaccel_decode_init, HWACCEL_VDPAU, AV_PIX_FMT_VDPAU,
> -      AV_HWDEVICE_TYPE_VDPAU },
> +      AV_HWDEVICE_TYPE_VDPAU, 0 },

Using designated initializers would probably be a good idea here.
diff mbox

Patch

diff --git a/ffmpeg.h b/ffmpeg.h
index fa81427471..6ea29ded11 100644
--- a/ffmpeg.h
+++ b/ffmpeg.h
@@ -76,6 +76,7 @@  typedef struct HWAccel {
     enum HWAccelID id;
     enum AVPixelFormat pix_fmt;
     enum AVHWDeviceType device_type;
+    int pix_fmt_is_default;
 } HWAccel;
 
 typedef struct HWDevice {
diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
index 6dc4ad43d2..b01dc0c895 100644
--- a/ffmpeg_opt.c
+++ b/ffmpeg_opt.c
@@ -68,31 +68,31 @@ 
 const HWAccel hwaccels[] = {
 #if HAVE_VDPAU_X11
     { "vdpau", hwaccel_decode_init, HWACCEL_VDPAU, AV_PIX_FMT_VDPAU,
-      AV_HWDEVICE_TYPE_VDPAU },
+      AV_HWDEVICE_TYPE_VDPAU, 0 },
 #endif
 #if HAVE_DXVA2_LIB
     { "dxva2", dxva2_init, HWACCEL_DXVA2, AV_PIX_FMT_DXVA2_VLD,
-      AV_HWDEVICE_TYPE_NONE },
+      AV_HWDEVICE_TYPE_NONE, 0 },
 #endif
 #if CONFIG_VDA
     { "vda",   videotoolbox_init,   HWACCEL_VDA,   AV_PIX_FMT_VDA,
-      AV_HWDEVICE_TYPE_NONE },
+      AV_HWDEVICE_TYPE_NONE, 0 },
 #endif
 #if CONFIG_VIDEOTOOLBOX
     { "videotoolbox",   videotoolbox_init,   HWACCEL_VIDEOTOOLBOX,   AV_PIX_FMT_VIDEOTOOLBOX,
-      AV_HWDEVICE_TYPE_NONE },
+      AV_HWDEVICE_TYPE_NONE, 0 },
 #endif
 #if CONFIG_LIBMFX
     { "qsv",   qsv_init,   HWACCEL_QSV,   AV_PIX_FMT_QSV,
-      AV_HWDEVICE_TYPE_NONE },
+      AV_HWDEVICE_TYPE_NONE, 0 },
 #endif
 #if CONFIG_VAAPI
     { "vaapi", hwaccel_decode_init, HWACCEL_VAAPI, AV_PIX_FMT_VAAPI,
-      AV_HWDEVICE_TYPE_VAAPI },
+      AV_HWDEVICE_TYPE_VAAPI, 0 },
 #endif
 #if CONFIG_CUVID
     { "cuvid", hwaccel_decode_init, HWACCEL_CUVID, AV_PIX_FMT_CUDA,
-      AV_HWDEVICE_TYPE_CUDA },
+      AV_HWDEVICE_TYPE_CUDA, 1 },
 #endif
     { 0 },
 };
@@ -708,6 +708,7 @@  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
         char *discard_str = NULL;
         const AVClass *cc = avcodec_get_class();
         const AVOption *discard_opt = av_opt_find(&cc, "skip_frame", NULL, 0, 0);
+        enum AVPixelFormat default_pix_fmt = AV_PIX_FMT_NONE;
 
         if (!ist)
             exit_program(1);
@@ -805,6 +806,8 @@  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
                     for (i = 0; hwaccels[i].name; i++) {
                         if (!strcmp(hwaccels[i].name, hwaccel)) {
                             ist->hwaccel_id = hwaccels[i].id;
+                            if (hwaccels[i].pix_fmt_is_default)
+                                default_pix_fmt = hwaccels[i].pix_fmt;
                             break;
                         }
                     }
@@ -837,7 +840,7 @@  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
                            "format: %s", hwaccel_output_format);
                 }
             } else {
-                ist->hwaccel_output_format = AV_PIX_FMT_NONE;
+                ist->hwaccel_output_format = default_pix_fmt;
             }
 
             ist->hwaccel_pix_fmt = AV_PIX_FMT_NONE;