diff mbox

[FFmpeg-devel] cuvid: add drop_second_field as input option and enable it by default

Message ID 58A0B144.8010500@email.cz
State New
Headers show

Commit Message

Miroslav Slugeň Feb. 12, 2017, 7:02 p.m. UTC 
There is no need to copy second field if deinterlacing, it doesn't make 
sense to use two times same image.

Comments

Timo Rothenpieler Feb. 12, 2017, 7:25 p.m. UTC | #1 
It does make sense, the deinterlacers convert 50 interlaced fields to 50
progressive frames, like for example yadif can do as well.

And disabling that functionality by default seems strange to me, as it's
clearly the superior mode of operation.

On 2/12/2017 8:02 PM, Miroslav Slugeň wrote:
> There is no need to copy second field if deinterlacing, it doesn't make
> sense to use two times same image.
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Miroslav Slugeň Feb. 12, 2017, 7:35 p.m. UTC | #2 
Thx for comment,

cuvid can't output 50fps deinterlaced output, both frames are same, and 
default behavior for yadif is to downcovert to half frame rate, so i 
think is very clever to do the same here.

Also if we return in decoder that input is framerate/2 when 
deinterlacing we should stay with that!

if (ctx->deint_mode != cudaVideoDeinterlaceMode_Weave)
     avctx->framerate = av_mul_q(avctx->framerate, (AVRational){2, 1});

Miroslav Slugeň
+420 724 825 885

Dne 12.2.2017 v 20:25 Timo Rothenpieler napsal(a):
> It does make sense, the deinterlacers convert 50 interlaced fields to 50
> progressive frames, like for example yadif can do as well.
>
> And disabling that functionality by default seems strange to me, as it's
> clearly the superior mode of operation.
>
> On 2/12/2017 8:02 PM, Miroslav Slugeň wrote:
>> There is no need to copy second field if deinterlacing, it doesn't make
>> sense to use two times same image.
>>
>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Hendrik Leppkes Feb. 12, 2017, 7:54 p.m. UTC | #3 
On Sun, Feb 12, 2017 at 8:35 PM, Miroslav Slugeň <thunder.m@email.cz> wrote:
> Thx for comment,
>
> cuvid can't output 50fps deinterlaced output, both frames are same, and
> default behavior for yadif is to downcovert to half frame rate, so i think
> is very clever to do the same here.
>

Thats not true, cuvid can output full 50fps deinterlaced output with
distinct different frames.

Please don't top post on this mailing list.

- Hendrik
Timo Rothenpieler Feb. 12, 2017, 7:56 p.m. UTC | #4 
On 2/12/2017 8:35 PM, Miroslav Slugeň wrote:
> Thx for comment,
> 
> cuvid can't output 50fps deinterlaced output, both frames are same, and
> default behavior for yadif is to downcovert to half frame rate, so i
> think is very clever to do the same here.
> 
> Also if we return in decoder that input is framerate/2 when
> deinterlacing we should stay with that!
> 
> if (ctx->deint_mode != cudaVideoDeinterlaceMode_Weave)
>     avctx->framerate = av_mul_q(avctx->framerate, (AVRational){2, 1});

That equation multiplies the framerate by two, as deinterlacing doubles
it from for example 25 to 50, producing two output frames for each input
frame with two fields.

Judging from the cuvid examples, extracting two frames per interlaced
frame is the intended way of using the decoder, and at least in Adaptive
mode the frames are definitely not the same.
Never gave bob to much of a look, maybe it doesn't support it there.
Miroslav Slugeň Feb. 12, 2017, 8:01 p.m. UTC | #5 
Dne 12.2.2017 v 20:56 Timo Rothenpieler napsal(a):
> On 2/12/2017 8:35 PM, Miroslav Slugeň wrote:
>> Thx for comment,
>>
>> cuvid can't output 50fps deinterlaced output, both frames are same, and
>> default behavior for yadif is to downcovert to half frame rate, so i
>> think is very clever to do the same here.
>>
>> Also if we return in decoder that input is framerate/2 when
>> deinterlacing we should stay with that!
>>
>> if (ctx->deint_mode != cudaVideoDeinterlaceMode_Weave)
>>      avctx->framerate = av_mul_q(avctx->framerate, (AVRational){2, 1});
> That equation multiplies the framerate by two, as deinterlacing doubles
> it from for example 25 to 50, producing two output frames for each input
> frame with two fields.
>
> Judging from the cuvid examples, extracting two frames per interlaced
> frame is the intended way of using the decoder, and at least in Adaptive
> mode the frames are definitely not the same.
> Never gave bob to much of a look, maybe it doesn't support it there.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Sorry, my fault, you are true, than please ignore this patch.

I will prepare updated one.

M.
diff mbox

Patch

From 73d8f9fa17c26125a5b03243284f67126c70d7a6 Mon Sep 17 00:00:00 2001
From: Miroslav Slugen <thunder.m@email.cz>
Date: Sun, 12 Feb 2017 20:00:51 +0100
Subject: [PATCH 1/1] cuvid: add drop_second_field as input option and enable
 it by default

---
 libavcodec/cuvid.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
index 9b35476..a2e125d 100644
--- a/libavcodec/cuvid.c
+++ b/libavcodec/cuvid.c
@@ -42,6 +42,7 @@  typedef struct CuvidContext
 
     char *cu_gpu;
     int nb_surfaces;
+    int drop_second_field;
 
     AVBufferRef *hwdevice;
     AVBufferRef *hwframe;
@@ -298,8 +299,10 @@  static int CUDAAPI cuvid_handle_picture_display(void *opaque, CUVIDPARSERDISPINF
     } else {
         parsed_frame.is_deinterlacing = 1;
         av_fifo_generic_write(ctx->frame_queue, &parsed_frame, sizeof(CuvidParsedFrame), NULL);
-        parsed_frame.second_field = 1;
-        av_fifo_generic_write(ctx->frame_queue, &parsed_frame, sizeof(CuvidParsedFrame), NULL);
+        if (!ctx->drop_second_field) {
+            parsed_frame.second_field = 1;
+            av_fifo_generic_write(ctx->frame_queue, &parsed_frame, sizeof(CuvidParsedFrame), NULL);
+        }
     }
 
     return 1;
@@ -930,6 +933,7 @@  static const AVOption options[] = {
     { "adaptive", "Adaptive deinterlacing",                  0, AV_OPT_TYPE_CONST, { .i64 = cudaVideoDeinterlaceMode_Adaptive }, 0, 0, VD, "deint" },
     { "gpu",      "GPU to be used for decoding", OFFSET(cu_gpu), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, VD },
     { "surfaces", "Maximum surfaces to be used for decoding", OFFSET(nb_surfaces), AV_OPT_TYPE_INT, { .i64 = 25 }, 0, INT_MAX, VD },
+    { "drop_second_field", "Drop second field when deinterlacing", OFFSET(drop_second_field), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, VD },
     { NULL }
 };
 
-- 
2.1.4