diff mbox series

[FFmpeg-devel,1/1] avcodec/nvenc: move lossless presets after new ones

Message ID 20210416115349.102120-2-martin.pulec@cesnet.cz
State New
Headers show
Series [FFmpeg-devel,1/1] avcodec/nvenc: move lossless presets after new ones | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Martin Pulec April 16, 2021, 11:53 a.m. UTC 
A check in nvenc.c checks for NV_ENC_CAPS_SUPPORT_LOSSLESS_ENCODE if
preset >= PRESET_LOSSLESS_DEFAULT which was true for the new presets. As
a result, the use of new presets (P1-P7) fail if the card doesn't
support lossless encoding.
---
 libavcodec/nvenc.h      | 4 ++--
 libavcodec/nvenc_h264.c | 2 +-
 libavcodec/nvenc_hevc.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

Comments

Timo Rothenpieler April 17, 2021, 5:25 p.m. UTC | #1 
I think I'd much prefer to stop relying on the order of this enum entirely.

nvenc_map_preset already sets a lossless flag. But it's called way too 
late for the capability check.
I see no reason the call to nvenc_map_preset can't be moved to much earlier.
Like, right into ff_nvenc_encode_init. Then the capacility check can 
just check for the NVENC_LOSSLESS flag instead of relying on the order 
of that enum.

I don't believe anything else relies on it, and if it does, it should 
changed to work like the above.
Timo Rothenpieler April 18, 2021, 10:04 a.m. UTC | #2 
pushed a refactor of the entire logic, since I realized that another 
piece of code (the options parsing logic) relies on the new presets 
being last.
diff mbox series

Patch

diff --git a/libavcodec/nvenc.h b/libavcodec/nvenc.h
index fefc5f7f0b..fd69b7809f 100644
--- a/libavcodec/nvenc.h
+++ b/libavcodec/nvenc.h
@@ -103,8 +103,6 @@  enum {
     PRESET_LOW_LATENCY_DEFAULT ,
     PRESET_LOW_LATENCY_HQ ,
     PRESET_LOW_LATENCY_HP,
-    PRESET_LOSSLESS_DEFAULT, // lossless presets must be the last ones
-    PRESET_LOSSLESS_HP,
 #ifdef NVENC_HAVE_NEW_PRESETS
     PRESET_P1,
     PRESET_P2,
@@ -114,6 +112,8 @@  enum {
     PRESET_P6,
     PRESET_P7,
 #endif
+    PRESET_LOSSLESS_DEFAULT, // lossless presets must be the last ones
+    PRESET_LOSSLESS_HP,
 };
 
 enum {
diff --git a/libavcodec/nvenc_h264.c b/libavcodec/nvenc_h264.c
index 4c2585876e..113840a672 100644
--- a/libavcodec/nvenc_h264.c
+++ b/libavcodec/nvenc_h264.c
@@ -27,7 +27,7 @@ 
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
 #ifdef NVENC_HAVE_NEW_PRESETS
-    { "preset",       "Set the encoding preset",            OFFSET(preset),       AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 },     PRESET_DEFAULT, PRESET_P7,          VE, "preset" },
+    { "preset",       "Set the encoding preset",            OFFSET(preset),       AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 },     PRESET_DEFAULT, PRESET_LOSSLESS_HP, VE, "preset" },
 #else
     { "preset",       "Set the encoding preset",            OFFSET(preset),       AV_OPT_TYPE_INT,   { .i64 = PRESET_MEDIUM }, PRESET_DEFAULT, PRESET_LOSSLESS_HP, VE, "preset" },
 #endif
diff --git a/libavcodec/nvenc_hevc.c b/libavcodec/nvenc_hevc.c
index 441e7871d2..46e4798a6f 100644
--- a/libavcodec/nvenc_hevc.c
+++ b/libavcodec/nvenc_hevc.c
@@ -27,7 +27,7 @@ 
 #define VE AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_ENCODING_PARAM
 static const AVOption options[] = {
 #ifdef NVENC_HAVE_NEW_PRESETS
-    { "preset",       "Set the encoding preset",            OFFSET(preset),       AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 },     PRESET_DEFAULT, PRESET_P7,          VE, "preset" },
+    { "preset",       "Set the encoding preset",            OFFSET(preset),       AV_OPT_TYPE_INT,   { .i64 = PRESET_P4 },     PRESET_DEFAULT, PRESET_LOSSLESS_HP, VE, "preset" },
 #else
     { "preset",       "Set the encoding preset",            OFFSET(preset),       AV_OPT_TYPE_INT,   { .i64 = PRESET_MEDIUM }, PRESET_DEFAULT, PRESET_LOSSLESS_HP, VE, "preset" },
 #endif