Re: [FFmpeg-devel] [PATCH] avfilter/avf_concat: fix EOF timestamp

2021-07-27 Thread Nicolas George 
Paul B Mahol (12021-07-27):
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/avf_concat.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/avf_concat.c b/libavfilter/avf_concat.c
> index cb46f52a04..9bbe896444 100644
> --- a/libavfilter/avf_concat.c
> +++ b/libavfilter/avf_concat.c
> @@ -397,12 +397,18 @@ static int activate(AVFilterContext *ctx)
>  /* Forward status change */
>  if (cat->cur_idx < ctx->nb_inputs) {
>  for (i = 0; i < ctx->nb_outputs; i++) {
> -ret = ff_inlink_acknowledge_status(ctx->inputs[cat->cur_idx + 
> i], , );
> +AVFilterLink *inlink = ctx->inputs[cat->cur_idx + i];
> +
> +ret = ff_inlink_acknowledge_status(inlink, , );
>  /* TODO use pts */
>  if (ret > 0) {
>  close_input(ctx, cat->cur_idx + i);
>  if (cat->cur_idx + ctx->nb_outputs >= ctx->nb_inputs) {
> -ff_outlink_set_status(ctx->outputs[i], status, pts);

> +int64_t eof_pts;
> +
> +eof_pts = cat->delta_ts;

You could merge these two lines.

> +eof_pts += av_rescale_q(pts, inlink->time_base, 
> ctx->outputs[i]->time_base);
> +ff_outlink_set_status(ctx->outputs[i], status, eof_pts);
>  }
>  if (!cat->nb_in_active) {
>  ret = flush_segment(ctx);

LGTM.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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".

[FFmpeg-devel] [PATCH] avfilter/avf_concat: fix EOF timestamp

2021-07-27 Thread Paul B Mahol 
Signed-off-by: Paul B Mahol 
---
 libavfilter/avf_concat.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/libavfilter/avf_concat.c b/libavfilter/avf_concat.c
index cb46f52a04..9bbe896444 100644
--- a/libavfilter/avf_concat.c
+++ b/libavfilter/avf_concat.c
@@ -397,12 +397,18 @@ static int activate(AVFilterContext *ctx)
 /* Forward status change */
 if (cat->cur_idx < ctx->nb_inputs) {
 for (i = 0; i < ctx->nb_outputs; i++) {
-ret = ff_inlink_acknowledge_status(ctx->inputs[cat->cur_idx + i], 
, );
+AVFilterLink *inlink = ctx->inputs[cat->cur_idx + i];
+
+ret = ff_inlink_acknowledge_status(inlink, , );
 /* TODO use pts */
 if (ret > 0) {
 close_input(ctx, cat->cur_idx + i);
 if (cat->cur_idx + ctx->nb_outputs >= ctx->nb_inputs) {
-ff_outlink_set_status(ctx->outputs[i], status, pts);
+int64_t eof_pts;
+
+eof_pts = cat->delta_ts;
+eof_pts += av_rescale_q(pts, inlink->time_base, 
ctx->outputs[i]->time_base);
+ff_outlink_set_status(ctx->outputs[i], status, eof_pts);
 }
 if (!cat->nb_in_active) {
 ret = flush_segment(ctx);
-- 
2.17.1

___
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".

Re: [FFmpeg-devel] [PATCH] avfilter/avf_concat: fix EOF timestamp

2021-07-26 Thread Nicolas George 
Paul B Mahol (12021-07-26):
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/avf_concat.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/libavfilter/avf_concat.c b/libavfilter/avf_concat.c
> index cb46f52a04..f7d3889dfa 100644
> --- a/libavfilter/avf_concat.c
> +++ b/libavfilter/avf_concat.c
> @@ -397,12 +397,16 @@ static int activate(AVFilterContext *ctx)
>  /* Forward status change */
>  if (cat->cur_idx < ctx->nb_inputs) {
>  for (i = 0; i < ctx->nb_outputs; i++) {
> -ret = ff_inlink_acknowledge_status(ctx->inputs[cat->cur_idx + 
> i], , );

> +AVFilterLink *inlink = ctx->inputs[cat->cur_idx + i];
> +
> +ret = ff_inlink_acknowledge_status(inlink, , );

Thanks for the change.

>  /* TODO use pts */
>  if (ret > 0) {
>  close_input(ctx, cat->cur_idx + i);
>  if (cat->cur_idx + ctx->nb_outputs >= ctx->nb_inputs) {
> -ff_outlink_set_status(ctx->outputs[i], status, pts);
> +ff_outlink_set_status(ctx->outputs[i], status,
> +  cat->delta_ts + av_rescale_q(pts, 
> inlink->time_base,
> +   
> ctx->outputs[i]->time_base));

My first review also said:

"and re-affect pts
in the line before rather than inlining the computation."

This multiply-indented formula is hard to read and would be very
annoying to maintain.

   pts = av_rescale_q(pts, inlink->time_base, 
ctx->outputs[i]->time_base);
   pts += cat->delta_ts;

would be much nicer.

>  }
>  if (!cat->nb_in_active) {
>  ret = flush_segment(ctx);

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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".

[FFmpeg-devel] [PATCH] avfilter/avf_concat: fix EOF timestamp

2021-07-26 Thread Paul B Mahol 
Signed-off-by: Paul B Mahol 
---
 libavfilter/avf_concat.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libavfilter/avf_concat.c b/libavfilter/avf_concat.c
index cb46f52a04..f7d3889dfa 100644
--- a/libavfilter/avf_concat.c
+++ b/libavfilter/avf_concat.c
@@ -397,12 +397,16 @@ static int activate(AVFilterContext *ctx)
 /* Forward status change */
 if (cat->cur_idx < ctx->nb_inputs) {
 for (i = 0; i < ctx->nb_outputs; i++) {
-ret = ff_inlink_acknowledge_status(ctx->inputs[cat->cur_idx + i], 
, );
+AVFilterLink *inlink = ctx->inputs[cat->cur_idx + i];
+
+ret = ff_inlink_acknowledge_status(inlink, , );
 /* TODO use pts */
 if (ret > 0) {
 close_input(ctx, cat->cur_idx + i);
 if (cat->cur_idx + ctx->nb_outputs >= ctx->nb_inputs) {
-ff_outlink_set_status(ctx->outputs[i], status, pts);
+ff_outlink_set_status(ctx->outputs[i], status,
+  cat->delta_ts + av_rescale_q(pts, 
inlink->time_base,
+   
ctx->outputs[i]->time_base));
 }
 if (!cat->nb_in_active) {
 ret = flush_segment(ctx);
-- 
2.17.1

___
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".

Re: [FFmpeg-devel] [PATCH] avfilter/avf_concat: fix EOF timestamp

2021-07-25 Thread Nicolas George 
Paul B Mahol (12021-07-25):
> Signed-off-by: Paul B Mahol 
> ---
>  libavfilter/avf_concat.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/avf_concat.c b/libavfilter/avf_concat.c
> index cb46f52a04..8bb5a5d618 100644
> --- a/libavfilter/avf_concat.c
> +++ b/libavfilter/avf_concat.c
> @@ -402,7 +402,10 @@ static int activate(AVFilterContext *ctx)
>  if (ret > 0) {
>  close_input(ctx, cat->cur_idx + i);
>  if (cat->cur_idx + ctx->nb_outputs >= ctx->nb_inputs) {

> -ff_outlink_set_status(ctx->outputs[i], status, pts);
> +ff_outlink_set_status(ctx->outputs[i], status,
> +  cat->delta_ts + av_rescale_q(pts,
> +   
> ctx->inputs[cat->cur_idx + i]->time_base,
> +   
> ctx->outputs[i]->time_base));

It looks valid, good catch, but ugly. Please add a local variable for
ctx->inputs[cat->cur_idx + i], which is now used twice, and re-affect pts
in the line before rather than inlining the computation.

>  }
>  if (!cat->nb_in_active) {
>  ret = flush_segment(ctx);

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
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".

[FFmpeg-devel] [PATCH] avfilter/avf_concat: fix EOF timestamp

2021-07-25 Thread Paul B Mahol 
Signed-off-by: Paul B Mahol 
---
 libavfilter/avf_concat.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libavfilter/avf_concat.c b/libavfilter/avf_concat.c
index cb46f52a04..8bb5a5d618 100644
--- a/libavfilter/avf_concat.c
+++ b/libavfilter/avf_concat.c
@@ -402,7 +402,10 @@ static int activate(AVFilterContext *ctx)
 if (ret > 0) {
 close_input(ctx, cat->cur_idx + i);
 if (cat->cur_idx + ctx->nb_outputs >= ctx->nb_inputs) {
-ff_outlink_set_status(ctx->outputs[i], status, pts);
+ff_outlink_set_status(ctx->outputs[i], status,
+  cat->delta_ts + av_rescale_q(pts,
+   
ctx->inputs[cat->cur_idx + i]->time_base,
+   
ctx->outputs[i]->time_base));
 }
 if (!cat->nb_in_active) {
 ret = flush_segment(ctx);
-- 
2.17.1

___
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".