| Message ID | 20190506124133.25596-1-onemda@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mon, May 6, 2019, 06:42 Paul B Mahol <onemda@gmail.com> wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > Makes ffplay display correct timestamps when seeking. > --- > libavfilter/af_atempo.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c > index bfdad7d76b..6a23d59641 100644 > --- a/libavfilter/af_atempo.c > +++ b/libavfilter/af_atempo.c > @@ -103,6 +103,9 @@ typedef struct ATempoContext { > // 1: output sample position > int64_t position[2]; > > + // first input timestamp, all other timestamps are offset by this one > + int64_t start_pts; > + > // sample format: > enum AVSampleFormat format; > > @@ -1055,6 +1058,7 @@ static int config_props(AVFilterLink *inlink) > enum AVSampleFormat format = inlink->format; > int sample_rate = (int)inlink->sample_rate; > > + atempo->start_pts = AV_NOPTS_VALUE; > return yae_reset(atempo, format, sample_rate, inlink->channels); > } > > @@ -1068,7 +1072,7 @@ static int push_samples(ATempoContext *atempo, > atempo->dst_buffer->nb_samples = n_out; > > // adjust the PTS: > - atempo->dst_buffer->pts = > + atempo->dst_buffer->pts = atempo->start_pts + > av_rescale_q(atempo->nsamples_out, > (AVRational){ 1, outlink->sample_rate }, > outlink->time_base); > @@ -1097,6 +1101,8 @@ static int filter_frame(AVFilterLink *inlink, > AVFrame *src_buffer) > const uint8_t *src = src_buffer->data[0]; > const uint8_t *src_end = src + n_in * atempo->stride; > > + if (atempo->start_pts == AV_NOPTS_VALUE) > + atempo->start_pts = src_buffer->pts; > while (src < src_end) { > if (!atempo->dst_buffer) { > atempo->dst_buffer = ff_get_audio_buffer(outlink, n_out); > Maybe ok. I am not sure how this would interact with seeking, have you tried that? I can check after work tomorrow, I don't think I can do it today as I will be out all evening. Thank you, Pavel.
On Mon, May 6, 2019, 07:28 Pavel Koshevoy <pkoshevoy@gmail.com> wrote: > > > On Mon, May 6, 2019, 06:42 Paul B Mahol <onemda@gmail.com> wrote: > >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> Makes ffplay display correct timestamps when seeking. >> --- >> libavfilter/af_atempo.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >> index bfdad7d76b..6a23d59641 100644 >> --- a/libavfilter/af_atempo.c >> +++ b/libavfilter/af_atempo.c >> @@ -103,6 +103,9 @@ typedef struct ATempoContext { >> // 1: output sample position >> int64_t position[2]; >> >> + // first input timestamp, all other timestamps are offset by this one >> + int64_t start_pts; >> + >> // sample format: >> enum AVSampleFormat format; >> >> @@ -1055,6 +1058,7 @@ static int config_props(AVFilterLink *inlink) >> enum AVSampleFormat format = inlink->format; >> int sample_rate = (int)inlink->sample_rate; >> >> + atempo->start_pts = AV_NOPTS_VALUE; >> return yae_reset(atempo, format, sample_rate, inlink->channels); >> } >> >> @@ -1068,7 +1072,7 @@ static int push_samples(ATempoContext *atempo, >> atempo->dst_buffer->nb_samples = n_out; >> >> // adjust the PTS: >> - atempo->dst_buffer->pts = >> + atempo->dst_buffer->pts = atempo->start_pts + >> av_rescale_q(atempo->nsamples_out, >> (AVRational){ 1, outlink->sample_rate }, >> outlink->time_base); >> @@ -1097,6 +1101,8 @@ static int filter_frame(AVFilterLink *inlink, >> AVFrame *src_buffer) >> const uint8_t *src = src_buffer->data[0]; >> const uint8_t *src_end = src + n_in * atempo->stride; >> >> + if (atempo->start_pts == AV_NOPTS_VALUE) >> + atempo->start_pts = src_buffer->pts; >> while (src < src_end) { >> if (!atempo->dst_buffer) { >> atempo->dst_buffer = ff_get_audio_buffer(outlink, n_out); >> > > > Maybe ok. I am not sure how this would interact with seeking, have you > tried that? I can check after work tomorrow, I don't think I can do it > today as I will be out all evening. > > Thank you, > Pavel. > The commit message answered my question, sorry. LGTM since you've checked seeking. Thank you, Pavel. > >
On 5/6/19 6:41 AM, Paul B Mahol wrote: > Signed-off-by: Paul B Mahol <onemda@gmail.com> > --- > Makes ffplay display correct timestamps when seeking. > --- > libavfilter/af_atempo.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c > index bfdad7d76b..6a23d59641 100644 > --- a/libavfilter/af_atempo.c > +++ b/libavfilter/af_atempo.c > @@ -103,6 +103,9 @@ typedef struct ATempoContext { > // 1: output sample position > int64_t position[2]; > > + // first input timestamp, all other timestamps are offset by this one > + int64_t start_pts; > + > // sample format: > enum AVSampleFormat format; > > @@ -1055,6 +1058,7 @@ static int config_props(AVFilterLink *inlink) > enum AVSampleFormat format = inlink->format; > int sample_rate = (int)inlink->sample_rate; > > + atempo->start_pts = AV_NOPTS_VALUE; > return yae_reset(atempo, format, sample_rate, inlink->channels); > } > > @@ -1068,7 +1072,7 @@ static int push_samples(ATempoContext *atempo, > atempo->dst_buffer->nb_samples = n_out; > > // adjust the PTS: > - atempo->dst_buffer->pts = > + atempo->dst_buffer->pts = atempo->start_pts + > av_rescale_q(atempo->nsamples_out, > (AVRational){ 1, outlink->sample_rate }, > outlink->time_base); > @@ -1097,6 +1101,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *src_buffer) > const uint8_t *src = src_buffer->data[0]; > const uint8_t *src_end = src + n_in * atempo->stride; > > + if (atempo->start_pts == AV_NOPTS_VALUE) > + atempo->start_pts = src_buffer->pts; > while (src < src_end) { > if (!atempo->dst_buffer) { > atempo->dst_buffer = ff_get_audio_buffer(outlink, n_out); I was able to confirm from the debugger that af_atempo.c:config_props gets called and atempo->start_pts is reset when seeking in ffplay, so the patch looks okay to me. Thank you, Pavel.
On 5/7/19 8:41 PM, Pavel Koshevoy wrote: > On 5/6/19 6:41 AM, Paul B Mahol wrote: >> Signed-off-by: Paul B Mahol <onemda@gmail.com> >> --- >> Makes ffplay display correct timestamps when seeking. >> --- >> libavfilter/af_atempo.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >> index bfdad7d76b..6a23d59641 100644 >> --- a/libavfilter/af_atempo.c >> +++ b/libavfilter/af_atempo.c >> @@ -103,6 +103,9 @@ typedef struct ATempoContext { >> // 1: output sample position >> int64_t position[2]; >> + // first input timestamp, all other timestamps are offset by this one >> + int64_t start_pts; >> + >> // sample format: >> enum AVSampleFormat format; >> @@ -1055,6 +1058,7 @@ static int config_props(AVFilterLink *inlink) >> enum AVSampleFormat format = inlink->format; >> int sample_rate = (int)inlink->sample_rate; >> + atempo->start_pts = AV_NOPTS_VALUE; >> return yae_reset(atempo, format, sample_rate, inlink->channels); >> } >> @@ -1068,7 +1072,7 @@ static int push_samples(ATempoContext *atempo, >> atempo->dst_buffer->nb_samples = n_out; >> // adjust the PTS: >> - atempo->dst_buffer->pts = >> + atempo->dst_buffer->pts = atempo->start_pts + >> av_rescale_q(atempo->nsamples_out, >> (AVRational){ 1, outlink->sample_rate }, >> outlink->time_base); >> @@ -1097,6 +1101,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame >> *src_buffer) >> const uint8_t *src = src_buffer->data[0]; >> const uint8_t *src_end = src + n_in * atempo->stride; >> + if (atempo->start_pts == AV_NOPTS_VALUE) >> + atempo->start_pts = src_buffer->pts; >> while (src < src_end) { >> if (!atempo->dst_buffer) { >> atempo->dst_buffer = ff_get_audio_buffer(outlink, n_out); > > > > I was able to confirm from the debugger that af_atempo.c:config_props gets > called and atempo->start_pts is reset when seeking in ffplay, so the patch > looks okay to me. > > Thank you, > Pavel. > However, it might be cleaner to move `atempo->start_pts = AV_NOPTS_VALUE;` from config_props to yae_clear
On 5/7/19 9:02 PM, Pavel Koshevoy wrote: > On 5/7/19 8:41 PM, Pavel Koshevoy wrote: >> On 5/6/19 6:41 AM, Paul B Mahol wrote: >>> Signed-off-by: Paul B Mahol <onemda@gmail.com> >>> --- >>> Makes ffplay display correct timestamps when seeking. >>> --- >>> libavfilter/af_atempo.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c >>> index bfdad7d76b..6a23d59641 100644 >>> --- a/libavfilter/af_atempo.c >>> +++ b/libavfilter/af_atempo.c >>> @@ -103,6 +103,9 @@ typedef struct ATempoContext { >>> // 1: output sample position >>> int64_t position[2]; >>> + // first input timestamp, all other timestamps are offset by this one >>> + int64_t start_pts; >>> + >>> // sample format: >>> enum AVSampleFormat format; >>> @@ -1055,6 +1058,7 @@ static int config_props(AVFilterLink *inlink) >>> enum AVSampleFormat format = inlink->format; >>> int sample_rate = (int)inlink->sample_rate; >>> + atempo->start_pts = AV_NOPTS_VALUE; >>> return yae_reset(atempo, format, sample_rate, inlink->channels); >>> } >>> @@ -1068,7 +1072,7 @@ static int push_samples(ATempoContext *atempo, >>> atempo->dst_buffer->nb_samples = n_out; >>> // adjust the PTS: >>> - atempo->dst_buffer->pts = >>> + atempo->dst_buffer->pts = atempo->start_pts + >>> av_rescale_q(atempo->nsamples_out, >>> (AVRational){ 1, outlink->sample_rate }, >>> outlink->time_base); >>> @@ -1097,6 +1101,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame >>> *src_buffer) >>> const uint8_t *src = src_buffer->data[0]; >>> const uint8_t *src_end = src + n_in * atempo->stride; >>> + if (atempo->start_pts == AV_NOPTS_VALUE) >>> + atempo->start_pts = src_buffer->pts; >>> while (src < src_end) { >>> if (!atempo->dst_buffer) { >>> atempo->dst_buffer = ff_get_audio_buffer(outlink, n_out); >> >> >> >> I was able to confirm from the debugger that af_atempo.c:config_props gets >> called and atempo->start_pts is reset when seeking in ffplay, so the patch >> looks okay to me. >> >> Thank you, >> Pavel. >> > > However, it might be cleaner to move `atempo->start_pts = AV_NOPTS_VALUE;` > from config_props to yae_clear > also, I am not certain that start_pts and output pts are expressed in the same time base... maybe start_pts should be rescaled to output timebase in filter_frame
btw, I don't know if there is already a way to do this with an existing utility
function... there probably is and I just don't know about it.
I wrote this debugging helper to help me verify input/output PTS in atempo, I
can send a patch if it's useful. Alternatively, I'd like to know what the right
way to get the same result would be with existing utility functions
static const char * hh_mm_ss_msec(char buf[15], AVRational b, int64_t t)
{
char * str = buf;
t = av_rescale_q(t, b, (AVRational){ 1, 1000 });
if (t < 0)
{
*str++ = '-';
t = -t;
}
// msec:
{
char * p = str + 8;
p[4] = 0;
p[3] = '0' + (t % 10);
t /= 10;
p[2] = '0' + (t % 10);
t /= 10;
p[1] = '0' + (t % 10);
t /= 10;
p[0] = '.';
}
// seconds:
{
int64_t v = t % 60;
char * p = str + 5;
p[2] = '0' + (v % 10);
v /= 10;
p[1] = '0' + v;
t /= 60;
p[0] = ':';
}
// minutes:
{
int64_t v = t % 60;
char * p = str + 2;
p[2] = '0' + (v % 10);
v /= 10;
p[1] = '0' + v;
t /= 60;
p[0] = ':';
}
// hours:
{
int64_t v = t % 100;
str[1] = '0' + (v % 10);
v /= 10;
str[0] = '0' + v;
}
return buf;
}
// elsewhere
char buf_src[15];
char buf_dst[15];
...
av_log(outlink->src, AV_LOG_WARNING,
"FIXME: pkoshevoy: atempo pts: %s -> %s\n",
hh_mm_ss_msec(buf_src,
(AVRational){ 1, outlink->sample_rate },
yae_curr_frag(atempo)->position[0] -
yae_curr_frag(atempo)->nsamples),
hh_mm_ss_msec(buf_dst,
outlink->time_base,
atempo->dst_buffer->pts));
// and the output looks like this:
[Parsed_atempo_2 @ 0x6a5ec0] FIXME: pkoshevoy: atempo pts: 00:00:29.991 -> 00:00:29.989
[Parsed_atempo_2 @ 0x6a5ec0] FIXME: pkoshevoy: atempo pts: 00:00:29.991 -> 00:00:30.002
[Parsed_atempo_2 @ 0x6a5ec0] FIXME: pkoshevoy: atempo pts: 00:00:30.043 -> 00:00:30.015
[Parsed_atempo_2 @ 0x6a5ec0] FIXME: pkoshevoy: atempo pts: 00:00:30.043 -> 00:00:30.028
[Parsed_atempo_2 @ 0x6a5ec0] FIXME: pkoshevoy: atempo pts: 00:00:30.093 -> 00:00:30.041
[Parsed_atempo_2 @ 0x6a5ec0] FIXME: pkoshevoy: atempo pts: 00:00:30.093 -> 00:00:30.054
diff --git a/libavfilter/af_atempo.c b/libavfilter/af_atempo.c index bfdad7d76b..6a23d59641 100644 --- a/libavfilter/af_atempo.c +++ b/libavfilter/af_atempo.c @@ -103,6 +103,9 @@ typedef struct ATempoContext { // 1: output sample position int64_t position[2]; + // first input timestamp, all other timestamps are offset by this one + int64_t start_pts; + // sample format: enum AVSampleFormat format; @@ -1055,6 +1058,7 @@ static int config_props(AVFilterLink *inlink) enum AVSampleFormat format = inlink->format; int sample_rate = (int)inlink->sample_rate; + atempo->start_pts = AV_NOPTS_VALUE; return yae_reset(atempo, format, sample_rate, inlink->channels); } @@ -1068,7 +1072,7 @@ static int push_samples(ATempoContext *atempo, atempo->dst_buffer->nb_samples = n_out; // adjust the PTS: - atempo->dst_buffer->pts = + atempo->dst_buffer->pts = atempo->start_pts + av_rescale_q(atempo->nsamples_out, (AVRational){ 1, outlink->sample_rate }, outlink->time_base); @@ -1097,6 +1101,8 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *src_buffer) const uint8_t *src = src_buffer->data[0]; const uint8_t *src_end = src + n_in * atempo->stride; + if (atempo->start_pts == AV_NOPTS_VALUE) + atempo->start_pts = src_buffer->pts; while (src < src_end) { if (!atempo->dst_buffer) { atempo->dst_buffer = ff_get_audio_buffer(outlink, n_out);
Signed-off-by: Paul B Mahol <onemda@gmail.com> --- Makes ffplay display correct timestamps when seeking. --- libavfilter/af_atempo.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)