[FFmpeg-devel] avfilter/af_atempo: offset all output timestamps by same amount of first input timestamp

Submitted by Paul B Mahol on May 6, 2019, 12:41 p.m.

Details

Message ID 20190506124133.25596-1-onemda@gmail.com
State New
Headers show

Commit Message

Paul B Mahol May 6, 2019, 12:41 p.m.
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(-)

Comments

Pavel Koshevoy May 6, 2019, 1:28 p.m.
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.
Pavel Koshevoy May 6, 2019, 1:32 p.m.
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.

>
>
Pavel Koshevoy May 8, 2019, 2:41 a.m.
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.
Pavel Koshevoy May 8, 2019, 3:02 a.m.
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
Pavel Koshevoy May 8, 2019, 3:14 a.m.
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
Pavel Koshevoy May 8, 2019, 4:28 a.m.
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

Patch hide | download patch | download mbox

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);