[FFmpeg-devel] [PATCH 0/3] Minor decklink output fixes

[FFmpeg-devel] [PATCH 0/3] Minor decklink output fixes

[Devin Heitmueller]

The following patch series contains several small fixes to the
decklink output support.

Devin Heitmueller (3):
  decklink: Don't take for granted that first frame to decklink output
    will be PTS 0
  decklink: Fix setting of last_pts to only be set for video
  decklink: Fix unused variable compiler warnings

 libavdevice/decklink_common.h |  1 +
 libavdevice/decklink_enc.cpp  | 13 +++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

-- 
2.35.1.655.ga68dfadae5

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

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

[FFmpeg-devel] [PATCH 1/3] decklink: Don't take for granted that first frame to decklink output will be PTS 0

[Devin Heitmueller]

The existing code assumed that the first frame received by the decklink
output would always be PTS zero.  However if running in other timing
modes than the default of CBR, items such as frame dropping at the
beginning may result in starting at a non-zero PTS.

For example, in our setup because we discard probing data and run
with "-vsync 2" the first video frame scheduled to the decklink
output will have a PTS around 170.  Scheduling frames too far into
the future will either fail or cause a backlog of frames scheduled
far enough into the future that the entire pipeline will stall.

Issue can be reproduced with the following command-line:

./ffmpeg -copyts -i foo.ts -f decklink -vcodec v210 -ac 2  'DeckLink Duo (4)'

Keep track of the PTS of the first frame received, so that when
we enable start playback we can provide that value to the decklink
driver.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavdevice/decklink_common.h | 1 +
 libavdevice/decklink_enc.cpp  | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 79d6ac5b38..088e165ee7 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -118,6 +118,7 @@ struct decklink_ctx {
 
     /* Status */
     int playback_started;
+    int64_t first_pts;
     int64_t last_pts;
     unsigned long frameCount;
     unsigned int dropped;
diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index fb686b9032..c3dc2c0cac 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -486,6 +486,9 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
     ctx->frames_buffer_available_spots--;
     pthread_mutex_unlock(&ctx->mutex);
 
+    if (ctx->first_pts == 0)
+        ctx->first_pts = pkt->pts;
+
     /* Schedule frame for playback. */
     hr = ctx->dlo->ScheduleVideoFrame((class IDeckLinkVideoFrame *) frame,
                                       pkt->pts * ctx->bmd_tb_num,
@@ -505,14 +508,14 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
                " Video may misbehave!\n");
 
     /* Preroll video frames. */
-    if (!ctx->playback_started && pkt->pts > ctx->frames_preroll) {
+    if (!ctx->playback_started && pkt->pts > (ctx->first_pts + ctx->frames_preroll)) {
         av_log(avctx, AV_LOG_DEBUG, "Ending audio preroll.\n");
         if (ctx->audio && ctx->dlo->EndAudioPreroll() != S_OK) {
             av_log(avctx, AV_LOG_ERROR, "Could not end audio preroll!\n");
             return AVERROR(EIO);
         }
         av_log(avctx, AV_LOG_DEBUG, "Starting scheduled playback.\n");
-        if (ctx->dlo->StartScheduledPlayback(0, ctx->bmd_tb_den, 1.0) != S_OK) {
+        if (ctx->dlo->StartScheduledPlayback(ctx->first_pts * ctx->bmd_tb_num, ctx->bmd_tb_den, 1.0) != S_OK) {
             av_log(avctx, AV_LOG_ERROR, "Could not start scheduled playback!\n");
             return AVERROR(EIO);
         }
-- 
2.35.1.655.ga68dfadae5

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

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

Re: [FFmpeg-devel] [PATCH 1/3] decklink: Don't take for granted that first frame to decklink output will be PTS 0

[Marton Balint]

On Thu, 23 Feb 2023, Devin Heitmueller wrote:

> The existing code assumed that the first frame received by the decklink
> output would always be PTS zero.  However if running in other timing
> modes than the default of CBR, items such as frame dropping at the
> beginning may result in starting at a non-zero PTS.
>
> For example, in our setup because we discard probing data and run
> with "-vsync 2" the first video frame scheduled to the decklink
> output will have a PTS around 170.  Scheduling frames too far into
> the future will either fail or cause a backlog of frames scheduled
> far enough into the future that the entire pipeline will stall.
>
> Issue can be reproduced with the following command-line:
>
> ./ffmpeg -copyts -i foo.ts -f decklink -vcodec v210 -ac 2  'DeckLink Duo (4)'
>
> Keep track of the PTS of the first frame received, so that when
> we enable start playback we can provide that value to the decklink
> driver.
>
> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
> ---
> libavdevice/decklink_common.h | 1 +
> libavdevice/decklink_enc.cpp  | 7 +++++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index 79d6ac5b38..088e165ee7 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -118,6 +118,7 @@ struct decklink_ctx {
>
>     /* Status */
>     int playback_started;
> +    int64_t first_pts;
>     int64_t last_pts;
>     unsigned long frameCount;
>     unsigned int dropped;
> diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
> index fb686b9032..c3dc2c0cac 100644
> --- a/libavdevice/decklink_enc.cpp
> +++ b/libavdevice/decklink_enc.cpp
> @@ -486,6 +486,9 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
>     ctx->frames_buffer_available_spots--;
>     pthread_mutex_unlock(&ctx->mutex);
>
> +    if (ctx->first_pts == 0)
> +        ctx->first_pts = pkt->pts;

And what if first packet pts is 0? Then the second packet pts will be 
assigned to first pts? Maybe you should use AV_NOPTS_VALUE for the default 
first_pts value and check for that instead.

Regards,
Marton

> +
>     /* Schedule frame for playback. */
>     hr = ctx->dlo->ScheduleVideoFrame((class IDeckLinkVideoFrame *) frame,
>                                       pkt->pts * ctx->bmd_tb_num,
> @@ -505,14 +508,14 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
>                " Video may misbehave!\n");
>
>     /* Preroll video frames. */
> -    if (!ctx->playback_started && pkt->pts > ctx->frames_preroll) {
> +    if (!ctx->playback_started && pkt->pts > (ctx->first_pts + ctx->frames_preroll)) {
>         av_log(avctx, AV_LOG_DEBUG, "Ending audio preroll.\n");
>         if (ctx->audio && ctx->dlo->EndAudioPreroll() != S_OK) {
>             av_log(avctx, AV_LOG_ERROR, "Could not end audio preroll!\n");
>             return AVERROR(EIO);
>         }
>         av_log(avctx, AV_LOG_DEBUG, "Starting scheduled playback.\n");
> -        if (ctx->dlo->StartScheduledPlayback(0, ctx->bmd_tb_den, 1.0) != S_OK) {
> +        if (ctx->dlo->StartScheduledPlayback(ctx->first_pts * ctx->bmd_tb_num, ctx->bmd_tb_den, 1.0) != S_OK) {
>             av_log(avctx, AV_LOG_ERROR, "Could not start scheduled playback!\n");
>             return AVERROR(EIO);
>         }
> -- 
> 2.35.1.655.ga68dfadae5
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH 1/3] decklink: Don't take for granted that first frame to decklink output will be PTS 0

[Devin Heitmueller]

On Tue, Feb 28, 2023 at 3:46 PM Marton Balint <cus@passwd.hu> wrote:
> And what if first packet pts is 0? Then the second packet pts will be
> assigned to first pts? Maybe you should use AV_NOPTS_VALUE for the default
> first_pts value and check for that instead.

That's a good point.  I never noticed that, but noticing that the
first frame got dropped is probably not something that would have been
immediately obvious.

In any case, I prefer your approach and will resubmit.

Thanks for the feedback,

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH 2/3] decklink: Fix setting of last_pts to only be set for video

[Devin Heitmueller]

The ff_decklink_write_packet() was always caching the last pts
received, to be used when calling StopScheduledPlayback().  However
because audio and video are on different timebases and the call to
StopScheduledPlayback() expects the video timebase, we'll end up
sending a weird value to the stop routine if the last packet
received contained audio.

Move the setting of last_pts to just be for the video stream.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavdevice/decklink_enc.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index c3dc2c0cac..f7daf37654 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -441,6 +441,8 @@ static int decklink_write_video_packet(AVFormatContext *avctx, AVPacket *pkt)
     uint32_t buffered;
     HRESULT hr;
 
+    ctx->last_pts = FFMAX(ctx->last_pts, pkt->pts);
+
     if (st->codecpar->codec_id == AV_CODEC_ID_WRAPPED_AVFRAME) {
         if (tmp->format != AV_PIX_FMT_UYVY422 ||
             tmp->width  != ctx->bmd_width ||
@@ -628,8 +630,6 @@ int ff_decklink_write_packet(AVFormatContext *avctx, AVPacket *pkt)
     struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
     AVStream *st = avctx->streams[pkt->stream_index];
 
-    ctx->last_pts = FFMAX(ctx->last_pts, pkt->pts);
-
     if      (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)
         return decklink_write_video_packet(avctx, pkt);
     else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO)
-- 
2.35.1.655.ga68dfadae5

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

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

[FFmpeg-devel] [PATCH 3/3] decklink: Fix unused variable compiler warnings

[Devin Heitmueller]

Due to refactoring, the ctx/cctx variables are never actually used
in ff_decklink_write_packet(), so just remove them.

Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com>
---
 libavdevice/decklink_enc.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index f7daf37654..160c6f837c 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -626,8 +626,6 @@ error:
 
 int ff_decklink_write_packet(AVFormatContext *avctx, AVPacket *pkt)
 {
-    struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
-    struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
     AVStream *st = avctx->streams[pkt->stream_index];
 
     if      (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)
-- 
2.35.1.655.ga68dfadae5

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

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