[PATCH] vf_overlay: add logic for avoiding overlaying frames with PTS > main frame PTS
Also add debug logging messages for helping tracking down similar issues. Fix trac ticket #467. --- libavfilter/vf_overlay.c | 41 +++++++++++++++++++++++++++++------------ 1 files changed, 29 insertions(+), 12 deletions(-) diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c index 062783e..9600061 100644 --- a/libavfilter/vf_overlay.c +++ b/libavfilter/vf_overlay.c @@ -25,6 +25,8 @@ * overlay one video on top of another */ +/* #define DEBUG */ + #include "avfilter.h" #include "libavutil/eval.h" #include "libavutil/avstring.h" @@ -75,7 +77,7 @@ typedef struct { uint8_t overlay_rgba_map[4]; uint8_t overlay_has_alpha; - AVFilterBufferRef *overpicref; + AVFilterBufferRef *overpicref, *overpicref_next; int main_pix_step[4]; ///< steps per pixel for each plane of the main output int overlay_pix_step[4]; ///< steps per pixel for each plane of the overlay @@ -144,6 +146,8 @@ static av_cold void uninit(AVFilterContext *ctx) if (over->overpicref) avfilter_unref_buffer(over->overpicref); + if (over->overpicref_next) + avfilter_unref_buffer(over->overpicref_next); } static int query_formats(AVFilterContext *ctx) @@ -302,22 +306,33 @@ static void start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref) AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0); AVFilterContext *ctx = inlink->dst; OverlayContext *over = ctx->priv; + av_unused AVFilterLink *outlink = ctx->outputs[0]; inlink->dst->outputs[0]->out_buf = outpicref; outpicref->pts = av_rescale_q(outpicref->pts, ctx->inputs[MAIN]->time_base, ctx->outputs[0]->time_base); if (!over->overpicref || over->overpicref->pts < outpicref->pts) { - AVFilterBufferRef *old = over->overpicref; - over->overpicref = NULL; - avfilter_request_frame(ctx->inputs[OVERLAY]); - if (over->overpicref) { - if (old) - avfilter_unref_buffer(old); - } else - over->overpicref = old; + if (!over->overpicref_next) + avfilter_request_frame(ctx->inputs[OVERLAY]); + + if (over->overpicref && over->overpicref_next && + over->overpicref_next->pts <= outpicref->pts) { + avfilter_unref_buffer(over->overpicref); + over->overpicref = over->overpicref_next; + over->overpicref_next = NULL; + } } +#ifdef DEBUG + av_log(ctx, AV_LOG_DEBUG, "main_pts:%"PRId64" main_pts_time:%f", + outpicref->pts, outpicref ->pts * av_q2d(outlink->time_base)); + if (over->overpicref) + av_log(ctx, AV_LOG_DEBUG, " over_pts:%"PRId64" over_pts_time:%f", + over->overpicref->pts, over->overpicref->pts * av_q2d(outlink->time_base)); + av_dlog(ctx, "\n"); +#endif + avfilter_start_frame(inlink->dst->outputs[0], outpicref); } @@ -326,9 +341,11 @@ static void start_frame_overlay(AVFilterLink *inlink, AVFilterBufferRef *inpicre AVFilterContext *ctx = inlink->dst; OverlayContext *over = ctx->priv; - over->overpicref = inpicref; - over->overpicref->pts = av_rescale_q(inpicref->pts, ctx->inputs[OVERLAY]->time_base, - ctx->outputs[0]->time_base); + inpicref->pts = av_rescale_q(inpicref->pts, ctx->inputs[OVERLAY]->time_base, + ctx->outputs[0]->time_base); + + if (!over->overpicref) over->overpicref = inpicref; + else over->overpicref_next = inpicref; } // divide by 255 and round to nearest -- 1.7.5.4
On 1/10/12, Stefano Sabatini <stefasab@gmail.com> wrote:
Also add debug logging messages for helping tracking down similar issues.
Fix trac ticket #467. --- libavfilter/vf_overlay.c | 41 +++++++++++++++++++++++++++++------------ 1 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c index 062783e..9600061 100644 --- a/libavfilter/vf_overlay.c +++ b/libavfilter/vf_overlay.c @@ -25,6 +25,8 @@ * overlay one video on top of another */
+/* #define DEBUG */
Is this really needed?
+ #include "avfilter.h" #include "libavutil/eval.h" #include "libavutil/avstring.h" @@ -75,7 +77,7 @@ typedef struct { uint8_t overlay_rgba_map[4]; uint8_t overlay_has_alpha;
- AVFilterBufferRef *overpicref; + AVFilterBufferRef *overpicref, *overpicref_next;
int main_pix_step[4]; ///< steps per pixel for each plane of the main output int overlay_pix_step[4]; ///< steps per pixel for each plane of the overlay @@ -144,6 +146,8 @@ static av_cold void uninit(AVFilterContext *ctx)
if (over->overpicref) avfilter_unref_buffer(over->overpicref); + if (over->overpicref_next) + avfilter_unref_buffer(over->overpicref_next); }
static int query_formats(AVFilterContext *ctx) @@ -302,22 +306,33 @@ static void start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref) AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0); AVFilterContext *ctx = inlink->dst; OverlayContext *over = ctx->priv; + av_unused AVFilterLink *outlink = ctx->outputs[0];
inlink->dst->outputs[0]->out_buf = outpicref; outpicref->pts = av_rescale_q(outpicref->pts, ctx->inputs[MAIN]->time_base, ctx->outputs[0]->time_base);
if (!over->overpicref || over->overpicref->pts < outpicref->pts) { - AVFilterBufferRef *old = over->overpicref; - over->overpicref = NULL; - avfilter_request_frame(ctx->inputs[OVERLAY]); - if (over->overpicref) { - if (old) - avfilter_unref_buffer(old); - } else - over->overpicref = old; + if (!over->overpicref_next) + avfilter_request_frame(ctx->inputs[OVERLAY]); + + if (over->overpicref && over->overpicref_next && + over->overpicref_next->pts <= outpicref->pts) { + avfilter_unref_buffer(over->overpicref); + over->overpicref = over->overpicref_next; + over->overpicref_next = NULL; + } }
+#ifdef DEBUG
Use av_dlog() instead.
+ av_log(ctx, AV_LOG_DEBUG, "main_pts:%"PRId64" main_pts_time:%f", + outpicref->pts, outpicref ->pts * av_q2d(outlink->time_base)); + if (over->overpicref) + av_log(ctx, AV_LOG_DEBUG, " over_pts:%"PRId64" over_pts_time:%f", + over->overpicref->pts, over->overpicref->pts * av_q2d(outlink->time_base)); + av_dlog(ctx, "\n"); +#endif + avfilter_start_frame(inlink->dst->outputs[0], outpicref); }
@@ -326,9 +341,11 @@ static void start_frame_overlay(AVFilterLink *inlink, AVFilterBufferRef *inpicre AVFilterContext *ctx = inlink->dst; OverlayContext *over = ctx->priv;
- over->overpicref = inpicref; - over->overpicref->pts = av_rescale_q(inpicref->pts, ctx->inputs[OVERLAY]->time_base, - ctx->outputs[0]->time_base); + inpicref->pts = av_rescale_q(inpicref->pts, ctx->inputs[OVERLAY]->time_base, + ctx->outputs[0]->time_base); + + if (!over->overpicref) over->overpicref = inpicref; + else over->overpicref_next = inpicref; }
// divide by 255 and round to nearest -- 1.7.5.4
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On date Tuesday 2012-01-10 00:30:12 +0000, Paul B Mahol encoded:
On 1/10/12, Stefano Sabatini <stefasab@gmail.com> wrote:
Also add debug logging messages for helping tracking down similar issues.
Fix trac ticket #467. --- libavfilter/vf_overlay.c | 41 +++++++++++++++++++++++++++++------------ 1 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c index 062783e..9600061 100644 --- a/libavfilter/vf_overlay.c +++ b/libavfilter/vf_overlay.c @@ -25,6 +25,8 @@ * overlay one video on top of another */
+/* #define DEBUG */
Is this really needed?
It's handy for me since I need just a few keystrokes for enabling/disabling it. Also it shows that there is debug code in the file.
+ #include "avfilter.h" #include "libavutil/eval.h" #include "libavutil/avstring.h" @@ -75,7 +77,7 @@ typedef struct { uint8_t overlay_rgba_map[4]; uint8_t overlay_has_alpha;
- AVFilterBufferRef *overpicref; + AVFilterBufferRef *overpicref, *overpicref_next;
int main_pix_step[4]; ///< steps per pixel for each plane of the main output int overlay_pix_step[4]; ///< steps per pixel for each plane of the overlay @@ -144,6 +146,8 @@ static av_cold void uninit(AVFilterContext *ctx)
if (over->overpicref) avfilter_unref_buffer(over->overpicref); + if (over->overpicref_next) + avfilter_unref_buffer(over->overpicref_next); }
static int query_formats(AVFilterContext *ctx) @@ -302,22 +306,33 @@ static void start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref) AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0); AVFilterContext *ctx = inlink->dst; OverlayContext *over = ctx->priv; + av_unused AVFilterLink *outlink = ctx->outputs[0];
inlink->dst->outputs[0]->out_buf = outpicref; outpicref->pts = av_rescale_q(outpicref->pts, ctx->inputs[MAIN]->time_base, ctx->outputs[0]->time_base);
if (!over->overpicref || over->overpicref->pts < outpicref->pts) { - AVFilterBufferRef *old = over->overpicref; - over->overpicref = NULL; - avfilter_request_frame(ctx->inputs[OVERLAY]); - if (over->overpicref) { - if (old) - avfilter_unref_buffer(old); - } else - over->overpicref = old; + if (!over->overpicref_next) + avfilter_request_frame(ctx->inputs[OVERLAY]); + + if (over->overpicref && over->overpicref_next && + over->overpicref_next->pts <= outpicref->pts) { + avfilter_unref_buffer(over->overpicref); + over->overpicref = over->overpicref_next; + over->overpicref_next = NULL; + } }
+#ifdef DEBUG
Use av_dlog() instead.
+ av_log(ctx, AV_LOG_DEBUG, "main_pts:%"PRId64" main_pts_time:%f", + outpicref->pts, outpicref ->pts * av_q2d(outlink->time_base)); + if (over->overpicref) + av_log(ctx, AV_LOG_DEBUG, " over_pts:%"PRId64" over_pts_time:%f", + over->overpicref->pts, over->overpicref->pts * av_q2d(outlink->time_base)); + av_dlog(ctx, "\n"); +#endif
Fixed. -- FFmpeg = Fundamental & Funny Minimalistic Portable Everlasting God
On 1/10/12, Stefano Sabatini <stefasab@gmail.com> wrote:
On date Tuesday 2012-01-10 00:30:12 +0000, Paul B Mahol encoded:
On 1/10/12, Stefano Sabatini <stefasab@gmail.com> wrote:
Also add debug logging messages for helping tracking down similar issues.
Fix trac ticket #467. --- libavfilter/vf_overlay.c | 41 +++++++++++++++++++++++++++++------------ 1 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c index 062783e..9600061 100644 --- a/libavfilter/vf_overlay.c +++ b/libavfilter/vf_overlay.c @@ -25,6 +25,8 @@ * overlay one video on top of another */
+/* #define DEBUG */
Is this really needed?
It's handy for me since I need just a few keystrokes for enabling/disabling it. Also it shows that there is debug code in the file.
+ #include "avfilter.h" #include "libavutil/eval.h" #include "libavutil/avstring.h" @@ -75,7 +77,7 @@ typedef struct { uint8_t overlay_rgba_map[4]; uint8_t overlay_has_alpha;
- AVFilterBufferRef *overpicref; + AVFilterBufferRef *overpicref, *overpicref_next;
int main_pix_step[4]; ///< steps per pixel for each plane of the main output int overlay_pix_step[4]; ///< steps per pixel for each plane of the overlay @@ -144,6 +146,8 @@ static av_cold void uninit(AVFilterContext *ctx)
if (over->overpicref) avfilter_unref_buffer(over->overpicref); + if (over->overpicref_next) + avfilter_unref_buffer(over->overpicref_next); }
static int query_formats(AVFilterContext *ctx) @@ -302,22 +306,33 @@ static void start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref) AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0); AVFilterContext *ctx = inlink->dst; OverlayContext *over = ctx->priv; + av_unused AVFilterLink *outlink = ctx->outputs[0];
inlink->dst->outputs[0]->out_buf = outpicref; outpicref->pts = av_rescale_q(outpicref->pts, ctx->inputs[MAIN]->time_base, ctx->outputs[0]->time_base);
if (!over->overpicref || over->overpicref->pts < outpicref->pts) { - AVFilterBufferRef *old = over->overpicref; - over->overpicref = NULL; - avfilter_request_frame(ctx->inputs[OVERLAY]); - if (over->overpicref) { - if (old) - avfilter_unref_buffer(old); - } else - over->overpicref = old; + if (!over->overpicref_next) + avfilter_request_frame(ctx->inputs[OVERLAY]); + + if (over->overpicref && over->overpicref_next && + over->overpicref_next->pts <= outpicref->pts) { + avfilter_unref_buffer(over->overpicref); + over->overpicref = over->overpicref_next; + over->overpicref_next = NULL; + } }
+#ifdef DEBUG
Use av_dlog() instead.
+ av_log(ctx, AV_LOG_DEBUG, "main_pts:%"PRId64" main_pts_time:%f", + outpicref->pts, outpicref ->pts * av_q2d(outlink->time_base)); + if (over->overpicref) + av_log(ctx, AV_LOG_DEBUG, " over_pts:%"PRId64" over_pts_time:%f", + over->overpicref->pts, over->overpicref->pts * av_q2d(outlink->time_base)); + av_dlog(ctx, "\n"); +#endif
Fixed.
Looks fine to me.
On date Wednesday 2012-02-01 00:43:01 +0000, Paul B Mahol encoded:
On 1/10/12, Stefano Sabatini <stefasab@gmail.com> wrote:
On date Tuesday 2012-01-10 00:30:12 +0000, Paul B Mahol encoded:
On 1/10/12, Stefano Sabatini <stefasab@gmail.com> wrote:
Also add debug logging messages for helping tracking down similar issues.
Fix trac ticket #467. --- libavfilter/vf_overlay.c | 41 +++++++++++++++++++++++++++++------------ 1 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c index 062783e..9600061 100644 --- a/libavfilter/vf_overlay.c +++ b/libavfilter/vf_overlay.c @@ -25,6 +25,8 @@ * overlay one video on top of another */
+/* #define DEBUG */
Is this really needed?
It's handy for me since I need just a few keystrokes for enabling/disabling it. Also it shows that there is debug code in the file.
+ #include "avfilter.h" #include "libavutil/eval.h" #include "libavutil/avstring.h" @@ -75,7 +77,7 @@ typedef struct { uint8_t overlay_rgba_map[4]; uint8_t overlay_has_alpha;
- AVFilterBufferRef *overpicref; + AVFilterBufferRef *overpicref, *overpicref_next;
int main_pix_step[4]; ///< steps per pixel for each plane of the main output int overlay_pix_step[4]; ///< steps per pixel for each plane of the overlay @@ -144,6 +146,8 @@ static av_cold void uninit(AVFilterContext *ctx)
if (over->overpicref) avfilter_unref_buffer(over->overpicref); + if (over->overpicref_next) + avfilter_unref_buffer(over->overpicref_next); }
static int query_formats(AVFilterContext *ctx) @@ -302,22 +306,33 @@ static void start_frame(AVFilterLink *inlink, AVFilterBufferRef *inpicref) AVFilterBufferRef *outpicref = avfilter_ref_buffer(inpicref, ~0); AVFilterContext *ctx = inlink->dst; OverlayContext *over = ctx->priv; + av_unused AVFilterLink *outlink = ctx->outputs[0];
inlink->dst->outputs[0]->out_buf = outpicref; outpicref->pts = av_rescale_q(outpicref->pts, ctx->inputs[MAIN]->time_base, ctx->outputs[0]->time_base);
if (!over->overpicref || over->overpicref->pts < outpicref->pts) { - AVFilterBufferRef *old = over->overpicref; - over->overpicref = NULL; - avfilter_request_frame(ctx->inputs[OVERLAY]); - if (over->overpicref) { - if (old) - avfilter_unref_buffer(old); - } else - over->overpicref = old; + if (!over->overpicref_next) + avfilter_request_frame(ctx->inputs[OVERLAY]); + + if (over->overpicref && over->overpicref_next && + over->overpicref_next->pts <= outpicref->pts) { + avfilter_unref_buffer(over->overpicref); + over->overpicref = over->overpicref_next; + over->overpicref_next = NULL; + } }
+#ifdef DEBUG
Use av_dlog() instead.
+ av_log(ctx, AV_LOG_DEBUG, "main_pts:%"PRId64" main_pts_time:%f", + outpicref->pts, outpicref ->pts * av_q2d(outlink->time_base)); + if (over->overpicref) + av_log(ctx, AV_LOG_DEBUG, " over_pts:%"PRId64" over_pts_time:%f", + over->overpicref->pts, over->overpicref->pts * av_q2d(outlink->time_base)); + av_dlog(ctx, "\n"); +#endif
Fixed.
Looks fine to me.
I was waiting for the timestamp utils patch, also I'm not sure this change is correct in case of undefined (NOTPS) timestamps, so the patch is temporarily suspended, thanks for the review anyway. -- FFmpeg = Friendly and Fostering Mega Pitiful Ecstatic Guru
On date Wednesday 2012-02-01 23:09:43 +0100, Stefano Sabatini encoded:
On date Wednesday 2012-02-01 00:43:01 +0000, Paul B Mahol encoded: [...]
Looks fine to me.
I was waiting for the timestamp utils patch, also I'm not sure this change is correct in case of undefined (NOTPS) timestamps, so the patch is temporarily suspended, thanks for the review anyway.
Patch updated, I'll push it tomorrow if I read no comments, as for the NOPTS issue I prefer to handle that with a separate patch. -- FFmpeg = Furious Friendly Mournful Portentous Empowered Gospel
On date Wednesday 2012-02-15 01:13:47 +0100, Stefano Sabatini encoded:
On date Wednesday 2012-02-01 23:09:43 +0100, Stefano Sabatini encoded:
On date Wednesday 2012-02-01 00:43:01 +0000, Paul B Mahol encoded: [...]
Looks fine to me.
I was waiting for the timestamp utils patch, also I'm not sure this change is correct in case of undefined (NOTPS) timestamps, so the patch is temporarily suspended, thanks for the review anyway.
Patch updated, I'll push it tomorrow if I read no comments, as for the NOPTS issue I prefer to handle that with a separate patch.
Applied. -- FFmpeg = Fierce Fancy Meaningful Powered Egregious Gnome
participants (2)
-
Paul B Mahol -
Stefano Sabatini