[FFmpeg-devel] avfilter/af_loudnorm: correctly initialize PTS

Submitted by Niklas Haas on Feb. 3, 2018, 2:22 p.m.

Details

Message ID 20180203142205.3550-1-ffmpeg@haasn.xyz
State Accepted
Commit 7c82e0f61e365296b094684fd92aea0fe05ceb93
Headers show

Commit Message

Niklas Haas Feb. 3, 2018, 2:22 p.m.
From: Niklas Haas <git@haasn.xyz>

Right now, the PTS always starts out as 0, which causes problems on a
seek or when inserting this filter mid-stream.

Initialize it instead to AV_NOPTS_VALUE and copy the PTS from the first
frame instead if this is the case.
---
 libavfilter/af_loudnorm.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Muhammad Faiz Feb. 3, 2018, 11:54 p.m.
On Sat, Feb 3, 2018 at 9:22 PM, Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.xyz>
>
> Right now, the PTS always starts out as 0, which causes problems on a
> seek or when inserting this filter mid-stream.
>
> Initialize it instead to AV_NOPTS_VALUE and copy the PTS from the first
> frame instead if this is the case.
> ---
>  libavfilter/af_loudnorm.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libavfilter/af_loudnorm.c b/libavfilter/af_loudnorm.c
> index a7f11cbe6e..314b25fa39 100644
> --- a/libavfilter/af_loudnorm.c
> +++ b/libavfilter/af_loudnorm.c
> @@ -431,6 +431,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
>          av_frame_copy_props(out, in);
>      }
>
> +    if (s->pts == AV_NOPTS_VALUE)
> +        s->pts = in->pts;
> +
>      out->pts = s->pts;
>      src = (const double *)in->data[0];
>      dst = (double *)out->data[0];
> @@ -763,7 +766,7 @@ static int config_input(AVFilterLink *inlink)
>          inlink->partial_buf_size = frame_size(inlink->sample_rate, 3000);
>      }
>
> -    s->pts =
> +    s->pts = AV_NOPTS_VALUE;
>      s->buf_index =
>      s->prev_buf_index =
>      s->limiter_buf_index = 0;
> --
> 2.16.1

Output pts ideally should be based on input pts in all cases
(not only in the first pts case), because input pts may be non-monotonic.
Paul B Mahol Feb. 4, 2018, 8:32 a.m.
On 2/4/18, Muhammad Faiz <mfcc64@gmail.com> wrote:
> On Sat, Feb 3, 2018 at 9:22 PM, Niklas Haas <ffmpeg@haasn.xyz> wrote:
>> From: Niklas Haas <git@haasn.xyz>
>>
>> Right now, the PTS always starts out as 0, which causes problems on a
>> seek or when inserting this filter mid-stream.
>>
>> Initialize it instead to AV_NOPTS_VALUE and copy the PTS from the first
>> frame instead if this is the case.
>> ---
>>  libavfilter/af_loudnorm.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavfilter/af_loudnorm.c b/libavfilter/af_loudnorm.c
>> index a7f11cbe6e..314b25fa39 100644
>> --- a/libavfilter/af_loudnorm.c
>> +++ b/libavfilter/af_loudnorm.c
>> @@ -431,6 +431,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
>> *in)
>>          av_frame_copy_props(out, in);
>>      }
>>
>> +    if (s->pts == AV_NOPTS_VALUE)
>> +        s->pts = in->pts;
>> +
>>      out->pts = s->pts;
>>      src = (const double *)in->data[0];
>>      dst = (double *)out->data[0];
>> @@ -763,7 +766,7 @@ static int config_input(AVFilterLink *inlink)
>>          inlink->partial_buf_size = frame_size(inlink->sample_rate, 3000);
>>      }
>>
>> -    s->pts =
>> +    s->pts = AV_NOPTS_VALUE;
>>      s->buf_index =
>>      s->prev_buf_index =
>>      s->limiter_buf_index = 0;
>> --
>> 2.16.1
>
> Output pts ideally should be based on input pts in all cases
> (not only in the first pts case), because input pts may be non-monotonic.

That have nothing to do with this patch.
Muhammad Faiz Feb. 4, 2018, 1:13 p.m.
On Sun, Feb 4, 2018 at 3:32 PM, Paul B Mahol <onemda@gmail.com> wrote:
> On 2/4/18, Muhammad Faiz <mfcc64@gmail.com> wrote:
>> On Sat, Feb 3, 2018 at 9:22 PM, Niklas Haas <ffmpeg@haasn.xyz> wrote:
>>> From: Niklas Haas <git@haasn.xyz>
>>>
>>> Right now, the PTS always starts out as 0, which causes problems on a
>>> seek or when inserting this filter mid-stream.
>>>
>>> Initialize it instead to AV_NOPTS_VALUE and copy the PTS from the first
>>> frame instead if this is the case.
>>> ---
>>>  libavfilter/af_loudnorm.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavfilter/af_loudnorm.c b/libavfilter/af_loudnorm.c
>>> index a7f11cbe6e..314b25fa39 100644
>>> --- a/libavfilter/af_loudnorm.c
>>> +++ b/libavfilter/af_loudnorm.c
>>> @@ -431,6 +431,9 @@ static int filter_frame(AVFilterLink *inlink, AVFrame
>>> *in)
>>>          av_frame_copy_props(out, in);
>>>      }
>>>
>>> +    if (s->pts == AV_NOPTS_VALUE)
>>> +        s->pts = in->pts;
>>> +
>>>      out->pts = s->pts;
>>>      src = (const double *)in->data[0];
>>>      dst = (double *)out->data[0];
>>> @@ -763,7 +766,7 @@ static int config_input(AVFilterLink *inlink)
>>>          inlink->partial_buf_size = frame_size(inlink->sample_rate, 3000);
>>>      }
>>>
>>> -    s->pts =
>>> +    s->pts = AV_NOPTS_VALUE;
>>>      s->buf_index =
>>>      s->prev_buf_index =
>>>      s->limiter_buf_index = 0;
>>> --
>>> 2.16.1
>>
>> Output pts ideally should be based on input pts in all cases
>> (not only in the first pts case), because input pts may be non-monotonic.
>
> That have nothing to do with this patch.

Of course, if ideal fix isn't trivial, this fix is enough.
Paul B Mahol Feb. 12, 2018, 6:35 p.m.
On 2/4/18, Muhammad Faiz <mfcc64@gmail.com> wrote:
> On Sun, Feb 4, 2018 at 3:32 PM, Paul B Mahol <onemda@gmail.com> wrote:
>> On 2/4/18, Muhammad Faiz <mfcc64@gmail.com> wrote:
>>> On Sat, Feb 3, 2018 at 9:22 PM, Niklas Haas <ffmpeg@haasn.xyz> wrote:
>>>> From: Niklas Haas <git@haasn.xyz>
>>>>
>>>> Right now, the PTS always starts out as 0, which causes problems on a
>>>> seek or when inserting this filter mid-stream.
>>>>
>>>> Initialize it instead to AV_NOPTS_VALUE and copy the PTS from the first
>>>> frame instead if this is the case.
>>>> ---
>>>>  libavfilter/af_loudnorm.c | 5 ++++-
>>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavfilter/af_loudnorm.c b/libavfilter/af_loudnorm.c
>>>> index a7f11cbe6e..314b25fa39 100644
>>>> --- a/libavfilter/af_loudnorm.c
>>>> +++ b/libavfilter/af_loudnorm.c
>>>> @@ -431,6 +431,9 @@ static int filter_frame(AVFilterLink *inlink,
>>>> AVFrame
>>>> *in)
>>>>          av_frame_copy_props(out, in);
>>>>      }
>>>>
>>>> +    if (s->pts == AV_NOPTS_VALUE)
>>>> +        s->pts = in->pts;
>>>> +
>>>>      out->pts = s->pts;
>>>>      src = (const double *)in->data[0];
>>>>      dst = (double *)out->data[0];
>>>> @@ -763,7 +766,7 @@ static int config_input(AVFilterLink *inlink)
>>>>          inlink->partial_buf_size = frame_size(inlink->sample_rate,
>>>> 3000);
>>>>      }
>>>>
>>>> -    s->pts =
>>>> +    s->pts = AV_NOPTS_VALUE;
>>>>      s->buf_index =
>>>>      s->prev_buf_index =
>>>>      s->limiter_buf_index = 0;
>>>> --
>>>> 2.16.1
>>>
>>> Output pts ideally should be based on input pts in all cases
>>> (not only in the first pts case), because input pts may be non-monotonic.
>>
>> That have nothing to do with this patch.
>
> Of course, if ideal fix isn't trivial, this fix is enough.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

So, if there are no more comments, I gonna apply this soon.

Patch hide | download patch | download mbox

diff --git a/libavfilter/af_loudnorm.c b/libavfilter/af_loudnorm.c
index a7f11cbe6e..314b25fa39 100644
--- a/libavfilter/af_loudnorm.c
+++ b/libavfilter/af_loudnorm.c
@@ -431,6 +431,9 @@  static int filter_frame(AVFilterLink *inlink, AVFrame *in)
         av_frame_copy_props(out, in);
     }
 
+    if (s->pts == AV_NOPTS_VALUE)
+        s->pts = in->pts;
+
     out->pts = s->pts;
     src = (const double *)in->data[0];
     dst = (double *)out->data[0];
@@ -763,7 +766,7 @@  static int config_input(AVFilterLink *inlink)
         inlink->partial_buf_size = frame_size(inlink->sample_rate, 3000);
     }
 
-    s->pts =
+    s->pts = AV_NOPTS_VALUE;
     s->buf_index =
     s->prev_buf_index =
     s->limiter_buf_index = 0;