diff mbox

[FFmpeg-devel] cuvid: Always calculate second field PTS from frame rate

Message ID 589B9C5A.7000101@email.cz
State New
Headers show

Commit Message

Miroslav Slugeň Feb. 8, 2017, 10:31 p.m. UTC
It is much more robust solution to always calculate second field (when 
deinterlacing) PTS from frame rate, then relating on previous PTS, 
because if there are B frames in input and some video frames are 
corrupted (dropped) it will calculate wrong PTS for second field and 
could hang on encoding in do_video_out.

Comments

Carl Eugen Hoyos Feb. 8, 2017, 10:59 p.m. UTC | #1
2017-02-08 23:31 GMT+01:00 Miroslav Slugeň <thunder.m@email.cz>:
> It is much more robust solution to always calculate second field
> (when deinterlacing) PTS from frame rate

Without looking at your patch, I would have guessed that not
every stream has a useful frame rate.
(It's 1000fps for asf input)

Carl Eugen
Miroslav Slugeň Feb. 9, 2017, 8:25 p.m. UTC | #2
Dne 8.2.2017 v 23:59 Carl Eugen Hoyos napsal(a):
> 2017-02-08 23:31 GMT+01:00 Miroslav Slugeň <thunder.m@email.cz>:
>> It is much more robust solution to always calculate second field
>> (when deinterlacing) PTS from frame rate
> Without looking at your patch, I would have guessed that not
> every stream has a useful frame rate.
> (It's 1000fps for asf input)
>
> Carl Eugen
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Ok, i didn't know that, so what about to calculate PTS diff only between 
non-B frames pictures?

I also have patch for dropping second field when deinterlacing, should i 
commit it?

M.
Miroslav Slugeň Feb. 9, 2017, 8:30 p.m. UTC | #3
Dne 9.2.2017 v 07:49 wm4 napsal(a):
> On Wed, 8 Feb 2017 23:31:54 +0100
> Miroslav Slugeň <thunder.m@email.cz> wrote:
>
>> It is much more robust solution to always calculate second field (when
>> deinterlacing) PTS from frame rate, then relating on previous PTS,
>> because if there are B frames in input and some video frames are
>> corrupted (dropped) it will calculate wrong PTS for second field and
>> could hang on encoding in do_video_out.
>>
>>
> Seems more like it'd make it less robust, because many files don't have
> a useful framerate set.
>
> I'm not sure why encoding is hanging (?), but that sounds like a
> different problem.
>
> Also, such reasoning should go into the commit message, instead of just
> the mail.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
I understand it now. I will try to find better solution, please ignore 
this patch.

Primary reason is when using cuvid as decoder with copyts we had alotof 
audio/video sync issues when there was B frame in input stream and some 
data was missing (gape in PTS).

M.
diff mbox

Patch

From f8a19eb5fb82fee0cce1e1d7248bec57bdea08ac Mon Sep 17 00:00:00 2001
From: Miroslav Slugen <thunder.m@email.cz>
Date: Wed, 8 Feb 2017 23:26:28 +0100
Subject: [PATCH 1/1] cuvid: Always calculate second field pts from frame rate

---
 libavcodec/cuvid.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/libavcodec/cuvid.c b/libavcodec/cuvid.c
index 9b35476..19c024d 100644
--- a/libavcodec/cuvid.c
+++ b/libavcodec/cuvid.c
@@ -51,7 +51,6 @@  typedef struct CuvidContext
     AVFifoBuffer *frame_queue;
 
     int deint_mode;
-    int64_t prev_pts;
 
     int internal_error;
     int decoder_flushing;
@@ -512,14 +511,7 @@  static int cuvid_output_frame(AVCodecContext *avctx, AVFrame *frame)
             frame->pts = parsed_frame.dispinfo.timestamp;
 
         if (parsed_frame.second_field) {
-            if (ctx->prev_pts == INT64_MIN) {
-                ctx->prev_pts = frame->pts;
-                frame->pts += (avctx->pkt_timebase.den * avctx->framerate.den) / (avctx->pkt_timebase.num * avctx->framerate.num);
-            } else {
-                int pts_diff = (frame->pts - ctx->prev_pts) / 2;
-                ctx->prev_pts = frame->pts;
-                frame->pts += pts_diff;
-            }
+            frame->pts += (avctx->pkt_timebase.den * avctx->framerate.den) / (avctx->pkt_timebase.num * avctx->framerate.num);
         }
 
         /* CUVIDs opaque reordering breaks the internal pkt logic.
@@ -853,8 +845,6 @@  static av_cold int cuvid_decode_init(AVCodecContext *avctx)
     if (ret < 0)
         goto error;
 
-    ctx->prev_pts = INT64_MIN;
-
     if (!avctx->pkt_timebase.num || !avctx->pkt_timebase.den)
         av_log(avctx, AV_LOG_WARNING, "Invalid pkt_timebase, passing timestamps as-is.\n");
 
@@ -913,7 +903,6 @@  static void cuvid_flush(AVCodecContext *avctx)
     if (ret < 0)
         goto error;
 
-    ctx->prev_pts = INT64_MIN;
     ctx->decoder_flushing = 0;
 
     return;
-- 
2.1.4