Re: [FFmpeg-devel] [PATCH] avdevice/decklink: add option to drop frames till timecode is seen

[Marton Balint]

On Wed, 13 Nov 2019, Gyan wrote:

As suggested in the review of an earlier patch*, this one drops frames till a 
TC is seen.


From 05555932db9d14dea215c1b0e5dcac4eac16cd6a Mon Sep 17 00:00:00 2001
From: Gyan Doshi <ffm...@gyani.pro>
Date: Mon, 9 Sep 2019 18:33:09 +0530
Subject: [PATCH] avdevice/decklink: add option to drop frames till timecode is
 seen

Option wait_for_tc only takes effect if tc_format is set
---
 libavdevice/decklink_common.h   |  1 +
 libavdevice/decklink_common_c.h |  1 +
 libavdevice/decklink_dec.cpp    | 12 ++++++++++++
 libavdevice/decklink_dec_c.c    |  1 +
 4 files changed, 15 insertions(+)

Docs update is missing for the new option as well as micro version bump.


diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 921818ba41..35422a300b 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -149,6 +149,7 @@ struct decklink_ctx {

     int channels;
     int audio_depth;
+    unsigned long tc_seen;    // used with option wait_for_tc
 };

 typedef enum { DIRECTION_IN, DIRECTION_OUT} decklink_direction_t;
diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
index ca85ec2504..b78630b5fc 100644
--- a/libavdevice/decklink_common_c.h
+++ b/libavdevice/decklink_common_c.h
@@ -58,6 +58,7 @@ struct decklink_cctx {
     int copyts;
     int64_t timestamp_align;
     int timing_offset;
+    int wait_for_tc;
 };

 #endif /* AVDEVICE_DECKLINK_COMMON_C_H */
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 4da9122bff..69be37ed64 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -784,6 +784,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
                             if (packed_metadata) {
                                 if (av_packet_add_side_data(&pkt, 
AV_PKT_DATA_STRINGS_METADATA, packed_metadata, metadata_len) < 0)
                                     av_freep(&packed_metadata);
+                                else if (!ctx->tc_seen)
+                                    ctx->tc_seen = ctx->frameCount;
                             }
                         }
                     }
@@ -793,6 +795,14 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
             }
         }

+        if (ctx->tc_format && cctx->wait_for_tc && !ctx->tc_seen) {
+
+            av_log(avctx, AV_LOG_WARNING, "No TC detected yet. wait_for_tc set. 
Dropping. \n");
+            av_log(avctx, AV_LOG_WARNING, "Frame received (#%lu) - "
+                        "- Frames dropped %u\n", ctx->frameCount, 
++ctx->dropped);
+            return S_OK;
+        }
+
         pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock, abs_wallclock, 
ctx->video_pts_source, ctx->video_st->time_base, &initial_video_pts, 
cctx->copyts);
         pkt.dts = pkt.pts;

@@ -880,6 +890,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
             videoFrame->AddRef();

         if (avpacket_queue_put(&ctx->queue, &pkt) < 0) {
+            if (ctx->tc_seen == ctx->frameCount)
+                ctx->tc_seen = 0;

I don't think this is needed, this can only happen in an ENOMEM case. Keep 
it if you feel strongly about it, but it looks a rather useless corner 
case to me.

LGTM if docs is added.

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".