| Message ID | 20170906202807.17675-1-tfoucu@gmail.com |
|---|---|
| State | Withdrawn |
| Headers | show |
Le decadi 20 fructidor, an CCXXV, Thierry Foucu a écrit : > --- > libavfilter/vf_fps.c | 40 +++++++++++++++++++++++++++++++++++----- > tests/ref/fate/filter-fps | 6 ++++++ > tests/ref/fate/filter-fps-r | 4 ++++ > 3 files changed, 45 insertions(+), 5 deletions(-) Already explained: no, pkt_duration is not part of the information taken into account by libavfilter. The correct way of obtaining the end timestamp is to use the link's timestamp after EOF. There is an old patch series to ensure it is accurate, but it has been delayed by bikeshedding. Regards,
On 9/6/17, Nicolas George <george@nsup.org> wrote: > Le decadi 20 fructidor, an CCXXV, Thierry Foucu a ecrit : >> --- >> libavfilter/vf_fps.c | 40 >> +++++++++++++++++++++++++++++++++++----- >> tests/ref/fate/filter-fps | 6 ++++++ >> tests/ref/fate/filter-fps-r | 4 ++++ >> 3 files changed, 45 insertions(+), 5 deletions(-) > > Already explained: no, pkt_duration is not part of the information taken > into account by libavfilter. Why? > The correct way of obtaining the end > timestamp is to use the link's timestamp after EOF. There is an old > patch series to ensure it is accurate, but it has been delayed by > bikeshedding. And why was it bikesheded?
Le decadi 20 fructidor, an CCXXV, Paul B Mahol a écrit : > Why? Several reasons that I have explained in the past. To make it short, the information there is not reliable, not filled by the applications, not updated by many filters and likely to be inconsistent. Is that enough? > And why was it bikesheded? For completely unrelated reason within the code of ffmpeg.c. Regards,
Hi Nicolas, On Wed, Sep 6, 2017 at 2:07 PM, Nicolas George <george@nsup.org> wrote: > Le decadi 20 fructidor, an CCXXV, Thierry Foucu a écrit : > > --- > > libavfilter/vf_fps.c | 40 ++++++++++++++++++++++++++++++ > +++++----- > > tests/ref/fate/filter-fps | 6 ++++++ > > tests/ref/fate/filter-fps-r | 4 ++++ > > 3 files changed, 45 insertions(+), 5 deletions(-) > > Already explained: no, pkt_duration is not part of the information taken > into account by libavfilter. The correct way of obtaining the end > timestamp is to use the link's timestamp after EOF. There is an old > patch series to ensure it is accurate, but it has been delayed by > bikeshedding. > > Sorry I did not know and I did not see any documentation in the header explaining which field of the AVFrame can/cannot be used by filter. If we cannot used the duration of the AVframe, how can we resolve the problem I'm trying to resolve. For example, if you take a video with a fps of 1 frame/second and you are trying to convert it to 30fps, the duration of the output file will be shorter by 1 second. Knowing the last frame duration allows us to pad the last video frame with the correct fps. You are saying there is an old patch series to ensure it. How far are you with it? With the patch you are talking about, will we be able to do what i'm trying to do with this patch? Regards. > Regards, > > -- > Nicolas George >
[ Ccing you, but no need to Cc me. ] Le primidi 21 fructidor, an CCXXV, Thierry Foucu a écrit : > Sorry I did not know and I did not see any documentation in the header > explaining which field of the AVFrame can/cannot be used by filter. Yes, it lacks documentation. > If we cannot used the duration of the AVframe, how can we resolve the > problem I'm trying to resolve. > For example, if you take a video with a fps of 1 frame/second and you are > trying to convert it to 30fps, the duration of the output file will be > shorter by 1 second. > Knowing the last frame duration allows us to pad the last video frame with > the correct fps. > > You are saying there is an old patch series to ensure it. How far are you > with it? > With the patch you are talking about, will we be able to do what i'm trying > to do with this patch? Yes, it is meant exactly for that. The patch series I was referring is this one: https://ffmpeg.org/pipermail/ffmpeg-devel/2017-September/215988.html https://patchwork.ffmpeg.org/patch/5024/ (and next) I thought I had cced you on one of the patches, but it may have been ignored. With this patch series, when a filter reaches EOF, the link->current_pts contains the timestamp of the end, including the duration of the last frame: from that, you can implement your patch in a very similar way. (Although vf_fps would need to be rewritten to take advantage of the link's FIFO, but that is more work, and not required immediately.) Regards,
On Thu, Sep 7, 2017 at 11:37 AM, Nicolas George <george@nsup.org> wrote: > [ Ccing you, but no need to Cc me. ] > > Le primidi 21 fructidor, an CCXXV, Thierry Foucu a écrit : > > Sorry I did not know and I did not see any documentation in the header > > explaining which field of the AVFrame can/cannot be used by filter. > > Yes, it lacks documentation. > > > If we cannot used the duration of the AVframe, how can we resolve the > > problem I'm trying to resolve. > > For example, if you take a video with a fps of 1 frame/second and you are > > trying to convert it to 30fps, the duration of the output file will be > > shorter by 1 second. > > Knowing the last frame duration allows us to pad the last video frame > with > > the correct fps. > > > > You are saying there is an old patch series to ensure it. How far are you > > with it? > > With the patch you are talking about, will we be able to do what i'm > trying > > to do with this patch? > > Yes, it is meant exactly for that. The patch series I was referring is > this one: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2017-September/215988.html > https://patchwork.ffmpeg.org/patch/5024/ > (and next) > > I thought I had cced you on one of the patches, but it may have been > ignored. > > Thanks Nicolas. > With this patch series, when a filter reaches EOF, the link->current_pts > contains the timestamp of the end, including the duration of the last > frame: from that, you can implement your patch in a very similar way. > As soon as your patches landed, I will re-work on my patch and send it to the list. > > (Although vf_fps would need to be rewritten to take advantage of the > link's FIFO, but that is more work, and not required immediately.) > any pointer on this? maybe i can give it a shot as well. > > Regards, > > -- > Nicolas George >
diff --git a/libavfilter/vf_fps.c b/libavfilter/vf_fps.c index 20ccd797d1..a231da415c 100644 --- a/libavfilter/vf_fps.c +++ b/libavfilter/vf_fps.c @@ -137,13 +137,43 @@ static int request_frame(AVFilterLink *outlink) AVFrame *buf; av_fifo_generic_read(s->fifo, &buf, sizeof(buf), NULL); - buf->pts = av_rescale_q(s->first_pts, ctx->inputs[0]->time_base, - outlink->time_base) + s->frames_out; + if (av_fifo_size(s->fifo)) { + buf->pts = av_rescale_q(s->first_pts, ctx->inputs[0]->time_base, + outlink->time_base) + s->frames_out; - if ((ret = ff_filter_frame(outlink, buf)) < 0) - return ret; + if ((ret = ff_filter_frame(outlink, buf)) < 0) + return ret; - s->frames_out++; + s->frames_out++; + } else { + /* This is the last frame, we may have to duplicate it to match + * the last frame duration */ + int j; + int delta = av_rescale_q_rnd(buf->pts + buf->pkt_duration - s->first_pts, + ctx->inputs[0]->time_base, + outlink->time_base, s->rounding) - s->frames_out ; + if (delta > 0 ) { + for (j = 0; j < delta; j++) { + AVFrame *dup = av_frame_clone(buf); + + av_log(ctx, AV_LOG_DEBUG, "Duplicating frame.\n"); + dup->pts = av_rescale_q(s->first_pts, ctx->inputs[0]->time_base, + outlink->time_base) + s->frames_out; + + if ((ret = ff_filter_frame(outlink, dup)) < 0) + return ret; + + s->frames_out++; + } + } else { + buf->pts = av_rescale_q(s->first_pts, ctx->inputs[0]->time_base, + outlink->time_base) + s->frames_out; + + if ((ret = ff_filter_frame(outlink, buf)) < 0) + return ret; + s->frames_out++; + } + } } return 0; } diff --git a/tests/ref/fate/filter-fps b/tests/ref/fate/filter-fps index 55712cfb1c..242fb04e85 100644 --- a/tests/ref/fate/filter-fps +++ b/tests/ref/fate/filter-fps @@ -85,3 +85,9 @@ 0, 79, 79, 1, 30576, 0xa2fcd06f 0, 80, 80, 1, 30576, 0xa2fcd06f 0, 81, 81, 1, 30576, 0xd4150aad +0, 82, 82, 1, 30576, 0xd4150aad +0, 83, 83, 1, 30576, 0xd4150aad +0, 84, 84, 1, 30576, 0xd4150aad +0, 85, 85, 1, 30576, 0xd4150aad +0, 86, 86, 1, 30576, 0xd4150aad +0, 87, 87, 1, 30576, 0xd4150aad diff --git a/tests/ref/fate/filter-fps-r b/tests/ref/fate/filter-fps-r index 826b1ed6c6..c1bc7d1547 100644 --- a/tests/ref/fate/filter-fps-r +++ b/tests/ref/fate/filter-fps-r @@ -72,3 +72,7 @@ 0, 79, 79, 1, 30576, 0xa2fcd06f 0, 80, 80, 1, 30576, 0xa2fcd06f 0, 82, 82, 1, 30576, 0xd4150aad +0, 83, 83, 1, 30576, 0xd4150aad +0, 84, 84, 1, 30576, 0xd4150aad +0, 85, 85, 1, 30576, 0xd4150aad +0, 86, 86, 1, 30576, 0xd4150aad