diff mbox series

[FFmpeg-devel] swresample fixes

Message ID CAPYw7P72E=6rM5W3Modq2qD+-SpcaFDE+N1Yec4jd6=zZZRpeA@mail.gmail.com
State New
Headers show
Series [FFmpeg-devel] swresample fixes | expand

Checks

Context Check Description
andriy/configure_x86 warning Failed to apply patch

Commit Message

Paul B Mahol Jan. 4, 2023, 4:59 p.m. UTC 
Patches attached.

Comments

James Almer Jan. 4, 2023, 5:02 p.m. UTC | #1 
On 1/4/2023 1:59 PM, Paul B Mahol wrote:
> From 0415ed37bee0c2b640920edad87ec927dda95fb5 Mon Sep 17 00:00:00 2001
> From: Paul B Mahol <onemda@gmail.com>
> Date: Wed, 4 Jan 2023 17:53:01 +0100
> Subject: [PATCH 2/3] swresample/swresample_frame: fix regression in detecting
>  changes
> 
> Do not overwrite return variable values, instead use different
> one for checking results.
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libswresample/swresample_frame.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/libswresample/swresample_frame.c b/libswresample/swresample_frame.c
> index 53ac487136..319ce045a1 100644
> --- a/libswresample/swresample_frame.c
> +++ b/libswresample/swresample_frame.c
> @@ -84,7 +84,7 @@ static int config_changed(SwrContext *s,
>                            const AVFrame *out, const AVFrame *in)
>  {
>      AVChannelLayout ch_layout = { 0 };
> -    int ret = 0;
> +    int ret = 0, iret;
>  
>      if (in) {
>  #if FF_API_OLD_CHANNEL_LAYOUT
> @@ -96,8 +96,8 @@ FF_DISABLE_DEPRECATION_WARNINGS
>  FF_ENABLE_DEPRECATION_WARNINGS
>          } else
>  #endif
> -        if ((ret = av_channel_layout_copy(&ch_layout, &in->ch_layout)) < 0)
> -            return ret;
> +        if ((iret = av_channel_layout_copy(&ch_layout, &in->ch_layout)) < 0)
> +            return iret;
>          if (av_channel_layout_compare(&s->in_ch_layout, &ch_layout) ||
>              s->in_sample_rate != in->sample_rate ||
>              s->in_sample_fmt  != in->format) {
> @@ -116,8 +116,8 @@ FF_DISABLE_DEPRECATION_WARNINGS
>  FF_ENABLE_DEPRECATION_WARNINGS
>          } else
>  #endif
> -        if ((ret = av_channel_layout_copy(&ch_layout, &out->ch_layout)) < 0)
> -            return ret;
> +        if ((iret = av_channel_layout_copy(&ch_layout, &out->ch_layout)) < 0)
> +            return iret;
>          if (av_channel_layout_compare(&s->out_ch_layout, &ch_layout) ||
>              s->out_sample_rate != out->sample_rate ||
>              s->out_sample_fmt  != out->format) {

Patch 2/3 LGTM, and it should be backported to 5.1.

Also, FWIW, these copies will be unnecessary after the old channel 
layout API is dropped, so this can be undone later.
Andreas Rheinhardt Jan. 4, 2023, 5:02 p.m. UTC | #2 
Paul B Mahol:
> diff --git a/libswresample/swresample_frame.c b/libswresample/swresample_frame.c
> index 53ac487136..319ce045a1 100644
> --- a/libswresample/swresample_frame.c
> +++ b/libswresample/swresample_frame.c
> @@ -84,7 +84,7 @@ static int config_changed(SwrContext *s,
>                            const AVFrame *out, const AVFrame *in)
>  {
>      AVChannelLayout ch_layout = { 0 };
> -    int ret = 0;
> +    int ret = 0, iret;
>  
>      if (in) {
>  #if FF_API_OLD_CHANNEL_LAYOUT
> @@ -96,8 +96,8 @@ FF_DISABLE_DEPRECATION_WARNINGS
>  FF_ENABLE_DEPRECATION_WARNINGS
>          } else
>  #endif
> -        if ((ret = av_channel_layout_copy(&ch_layout, &in->ch_layout)) < 0)
> -            return ret;
> +        if ((iret = av_channel_layout_copy(&ch_layout, &in->ch_layout)) < 0)
> +            return iret;
>          if (av_channel_layout_compare(&s->in_ch_layout, &ch_layout) ||
>              s->in_sample_rate != in->sample_rate ||
>              s->in_sample_fmt  != in->format) {
> @@ -116,8 +116,8 @@ FF_DISABLE_DEPRECATION_WARNINGS
>  FF_ENABLE_DEPRECATION_WARNINGS
>          } else
>  #endif
> -        if ((ret = av_channel_layout_copy(&ch_layout, &out->ch_layout)) < 0)
> -            return ret;
> +        if ((iret = av_channel_layout_copy(&ch_layout, &out->ch_layout)) < 0)
> +            return iret;
>          if (av_channel_layout_compare(&s->out_ch_layout, &ch_layout) ||
>              s->out_sample_rate != out->sample_rate ||
>              s->out_sample_fmt  != out->format) {

Using err instead of iret seems better.

- Andreas
Andreas Rheinhardt Jan. 4, 2023, 5:18 p.m. UTC | #3 
Paul B Mahol:
> diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> index 971c861d0e..7923377c8c 100644
> --- a/libavfilter/af_aresample.c
> +++ b/libavfilter/af_aresample.c
> @@ -209,18 +209,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      } else {
>          outsamplesref->pts  = AV_NOPTS_VALUE;
>      }
> -    n_out = swr_convert(aresample->swr, outsamplesref->extended_data, n_out,
> -                                 (void *)insamplesref->extended_data, n_in);
> -    if (n_out <= 0) {
> +    ret = swr_convert_frame(aresample->swr, outsamplesref,
> +                            (void *)insamplesref);

Don't know whether the actual change has advantages, but you should not
cast here. The cast above exists because there is no automatic cast
T**->const T** (it is actually unsafe; we should change swr_convert() to
accept const uint8_t *const * for in and uint8_t *const * for out at the
next major version bump, but even then C requires the cast). There is no
reason for a cast with the new code.

> +    if (ret < 0) {
>          av_frame_free(&outsamplesref);
>          av_frame_free(&insamplesref);
> -        return 0;
> +        return ret;
>      }
>  
>      aresample->more_data = outsamplesref->nb_samples == n_out; // Indicate that there is probably more data in our buffers
>  
> -    outsamplesref->nb_samples  = n_out;
> -
>      ret = ff_filter_frame(outlink, outsamplesref);
>      av_frame_free(&insamplesref);
>      return ret;
Paul B Mahol Jan. 4, 2023, 5:25 p.m. UTC | #4 
On Wed, Jan 4, 2023 at 6:18 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Paul B Mahol:
> > diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> > index 971c861d0e..7923377c8c 100644
> > --- a/libavfilter/af_aresample.c
> > +++ b/libavfilter/af_aresample.c
> > @@ -209,18 +209,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >      } else {
> >          outsamplesref->pts  = AV_NOPTS_VALUE;
> >      }
> > -    n_out = swr_convert(aresample->swr, outsamplesref->extended_data,
> n_out,
> > -                                 (void *)insamplesref->extended_data,
> n_in);
> > -    if (n_out <= 0) {
> > +    ret = swr_convert_frame(aresample->swr, outsamplesref,
> > +                            (void *)insamplesref);
>
> Don't know whether the actual change has advantages, but you should not
>

It adds better support for runtime change of input/output sample rates.
Instead of re-initializing whole filter graph.


> cast here. The cast above exists because there is no automatic cast
> T**->const T** (it is actually unsafe; we should change swr_convert() to
> accept const uint8_t *const * for in and uint8_t *const * for out at the
> next major version bump, but even then C requires the cast). There is no
> reason for a cast with the new code.
>
> > +    if (ret < 0) {
> >          av_frame_free(&outsamplesref);
> >          av_frame_free(&insamplesref);
> > -        return 0;
> > +        return ret;
> >      }
> >
> >      aresample->more_data = outsamplesref->nb_samples == n_out; //
> Indicate that there is probably more data in our buffers
> >
> > -    outsamplesref->nb_samples  = n_out;
> > -
> >      ret = ff_filter_frame(outlink, outsamplesref);
> >      av_frame_free(&insamplesref);
> >      return ret;
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Andreas Rheinhardt Jan. 4, 2023, 5:26 p.m. UTC | #5 
Paul B Mahol:
> diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> index 7923377c8c..2744388f75 100644
> --- a/libavfilter/af_aresample.c
> +++ b/libavfilter/af_aresample.c
> @@ -209,8 +209,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      } else {
>          outsamplesref->pts  = AV_NOPTS_VALUE;
>      }
> +again:
>      ret = swr_convert_frame(aresample->swr, outsamplesref,
>                              (void *)insamplesref);
> +    if (ret & (AVERROR_INPUT_CHANGED | AVERROR_OUTPUT_CHANGED)) {

This is wrong: There are lots of errors besides AVERROR_INPUT_CHANGED,
AVERROR_OUTPUT_CHANGED and AVERROR_INPUT_CHANGED |
AVERROR_OUTPUT_CHANGED for which this condition is true. See also ticket
#9343.

> +        swr_close(aresample->swr);
> +        goto again;
> +    }
> +
>      if (ret < 0) {
>          av_frame_free(&outsamplesref);
>          av_frame_free(&insamplesref);
Paul B Mahol Jan. 4, 2023, 5:35 p.m. UTC | #6 
On Wed, Jan 4, 2023 at 6:26 PM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Paul B Mahol:
> > diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> > index 7923377c8c..2744388f75 100644
> > --- a/libavfilter/af_aresample.c
> > +++ b/libavfilter/af_aresample.c
> > @@ -209,8 +209,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >      } else {
> >          outsamplesref->pts  = AV_NOPTS_VALUE;
> >      }
> > +again:
> >      ret = swr_convert_frame(aresample->swr, outsamplesref,
> >                              (void *)insamplesref);
> > +    if (ret & (AVERROR_INPUT_CHANGED | AVERROR_OUTPUT_CHANGED)) {
>
> This is wrong: There are lots of errors besides AVERROR_INPUT_CHANGED,
> AVERROR_OUTPUT_CHANGED and AVERROR_INPUT_CHANGED |
> AVERROR_OUTPUT_CHANGED for which this condition is true. See also ticket
> #9343.
>

So what you propose?


>
> > +        swr_close(aresample->swr);
> > +        goto again;
> > +    }
> > +
> >      if (ret < 0) {
> >          av_frame_free(&outsamplesref);
> >          av_frame_free(&insamplesref);
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Andreas Rheinhardt Jan. 4, 2023, 5:37 p.m. UTC | #7 
Paul B Mahol:
> On Wed, Jan 4, 2023 at 6:26 PM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
> 
>> Paul B Mahol:
>>> diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
>>> index 7923377c8c..2744388f75 100644
>>> --- a/libavfilter/af_aresample.c
>>> +++ b/libavfilter/af_aresample.c
>>> @@ -209,8 +209,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>>      } else {
>>>          outsamplesref->pts  = AV_NOPTS_VALUE;
>>>      }
>>> +again:
>>>      ret = swr_convert_frame(aresample->swr, outsamplesref,
>>>                              (void *)insamplesref);
>>> +    if (ret & (AVERROR_INPUT_CHANGED | AVERROR_OUTPUT_CHANGED)) {
>>
>> This is wrong: There are lots of errors besides AVERROR_INPUT_CHANGED,
>> AVERROR_OUTPUT_CHANGED and AVERROR_INPUT_CHANGED |
>> AVERROR_OUTPUT_CHANGED for which this condition is true. See also ticket
>> #9343.
>>
> 
> So what you propose?
> 

The only check way I see to do this is to check for ret ==
AVERROR_INPUT_CHANGED || ret == AVERROR_OUTPUT_CHANGED || ret ==
(AVERROR_INPUT_CHANGED | AVERROR_OUTPUT_CHANGED) (the latter value is
equal to AVERROR_INPUT_CHANGED). This should be done in the ret < 0
codepath.

("lots of errors" is btw an understatement: All errors are < 0 and
therefore have the sign bit set.)

> 
>>
>>> +        swr_close(aresample->swr);
>>> +        goto again;
>>> +    }
>>> +
>>>      if (ret < 0) {
>>>          av_frame_free(&outsamplesref);
>>>          av_frame_free(&insamplesref);
>>
>>
Michael Niedermayer Jan. 5, 2023, 8:21 p.m. UTC | #8 
On Wed, Jan 04, 2023 at 05:59:14PM +0100, Paul B Mahol wrote:
> Patches attached.

[...]
>  af_aresample.c |   10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> e517e6bf054deb40fe9ff16f6ce6585c6a1fb284  0001-avfilter-af_aresample-switch-to-convert-frame-API.patch
> From fe951a82dc6c75eced5b306a11ea5462a245c4c3 Mon Sep 17 00:00:00 2001
> From: Paul B Mahol <onemda@gmail.com>
> Date: Wed, 4 Jan 2023 17:13:16 +0100
> Subject: [PATCH 1/3] avfilter/af_aresample: switch to convert frame API
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavfilter/af_aresample.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> index 971c861d0e..7923377c8c 100644
> --- a/libavfilter/af_aresample.c
> +++ b/libavfilter/af_aresample.c
> @@ -209,18 +209,16 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      } else {
>          outsamplesref->pts  = AV_NOPTS_VALUE;
>      }
> -    n_out = swr_convert(aresample->swr, outsamplesref->extended_data, n_out,
> -                                 (void *)insamplesref->extended_data, n_in);
> -    if (n_out <= 0) {
> +    ret = swr_convert_frame(aresample->swr, outsamplesref,
> +                            (void *)insamplesref);
> +    if (ret < 0) {
>          av_frame_free(&outsamplesref);
>          av_frame_free(&insamplesref);
> -        return 0;
> +        return ret;
>      }
>  
>      aresample->more_data = outsamplesref->nb_samples == n_out; // Indicate that there is probably more data in our buffers
>  
> -    outsamplesref->nb_samples  = n_out;
> -
>      ret = ff_filter_frame(outlink, outsamplesref);
>      av_frame_free(&insamplesref);
>      return ret;
> -- 
> 2.37.2

This breaks libswresample with soxr
exmple:
ffmpeg  -loglevel error -y -f s32le -acodec pcm_s32le -ar 96000 -ac 1 -i ref-96000.raw -af aresample=osr=44100:resampler=soxr:cutoff=.903:precision=24:cheby=1 -f s32le -acodec pcm_s32le 96000-44100.raw

before it shows no errors
after it shows:
...
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 285242 >= 285242
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 286690 >= 286690
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 288138 >= 288138
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 289586 >= 289586
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 291034 >= 291034
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 291758 >= 291758
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 293206 >= 293206
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 294654 >= 294654
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 296102 >= 296102
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 297550 >= 297550
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 298998 >= 298998
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 300446 >= 300446
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 301170 >= 301170
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 302617 >= 302617
[s32le @ 0x55b8463eb580] Application provided invalid, non monotonically increasing dts to muxer in stream 0: 304065 >= 304065
...

[...]
Michael Niedermayer Jan. 5, 2023, 8:34 p.m. UTC | #9 
On Wed, Jan 04, 2023 at 05:59:14PM +0100, Paul B Mahol wrote:
> Patches attached.

>  af_aresample.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 94dacb46103e2bb9fbb6e1ca40675243d15069cd  0003-avfilter-af_aresample-if-frame-parameters-change-upd.patch
> From 3959bcb707f52339bac41acc9aec856cad3aced1 Mon Sep 17 00:00:00 2001
> From: Paul B Mahol <onemda@gmail.com>
> Date: Wed, 4 Jan 2023 17:55:10 +0100
> Subject: [PATCH 3/3] avfilter/af_aresample: if frame parameters change update
>  swr context
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavfilter/af_aresample.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> index 7923377c8c..2744388f75 100644
> --- a/libavfilter/af_aresample.c
> +++ b/libavfilter/af_aresample.c
> @@ -209,8 +209,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
>      } else {
>          outsamplesref->pts  = AV_NOPTS_VALUE;
>      }
> +again:
>      ret = swr_convert_frame(aresample->swr, outsamplesref,
>                              (void *)insamplesref);
> +    if (ret & (AVERROR_INPUT_CHANGED | AVERROR_OUTPUT_CHANGED)) {
> +        swr_close(aresample->swr);
> +        goto again;
> +    }
> +
>      if (ret < 0) {
>          av_frame_free(&outsamplesref);
>          av_frame_free(&insamplesref);

Are you sure this is not missing some flushing of internal samples ?

thx

[...]
Paul B Mahol Jan. 5, 2023, 8:44 p.m. UTC | #10 
On Thu, Jan 5, 2023 at 9:34 PM Michael Niedermayer <michael@niedermayer.cc>
wrote:

> On Wed, Jan 04, 2023 at 05:59:14PM +0100, Paul B Mahol wrote:
> > Patches attached.
>
> >  af_aresample.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> > 94dacb46103e2bb9fbb6e1ca40675243d15069cd
> 0003-avfilter-af_aresample-if-frame-parameters-change-upd.patch
> > From 3959bcb707f52339bac41acc9aec856cad3aced1 Mon Sep 17 00:00:00 2001
> > From: Paul B Mahol <onemda@gmail.com>
> > Date: Wed, 4 Jan 2023 17:55:10 +0100
> > Subject: [PATCH 3/3] avfilter/af_aresample: if frame parameters change
> update
> >  swr context
> >
> > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > ---
> >  libavfilter/af_aresample.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> > index 7923377c8c..2744388f75 100644
> > --- a/libavfilter/af_aresample.c
> > +++ b/libavfilter/af_aresample.c
> > @@ -209,8 +209,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >      } else {
> >          outsamplesref->pts  = AV_NOPTS_VALUE;
> >      }
> > +again:
> >      ret = swr_convert_frame(aresample->swr, outsamplesref,
> >                              (void *)insamplesref);
> > +    if (ret & (AVERROR_INPUT_CHANGED | AVERROR_OUTPUT_CHANGED)) {
> > +        swr_close(aresample->swr);
> > +        goto again;
> > +    }
> > +
> >      if (ret < 0) {
> >          av_frame_free(&outsamplesref);
> >          av_frame_free(&insamplesref);
>
> Are you sure this is not missing some flushing of internal samples ?
>

Nope, point is not to flush all internal samples at all, just enough so
clicks do not happen.
Clicks happen anyway so need to check why.



>
> thx
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> No snowflake in an avalanche ever feels responsible. -- Voltaire
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
Michael Niedermayer Jan. 5, 2023, 8:59 p.m. UTC | #11 
On Thu, Jan 05, 2023 at 09:44:46PM +0100, Paul B Mahol wrote:
> On Thu, Jan 5, 2023 at 9:34 PM Michael Niedermayer <michael@niedermayer.cc>
> wrote:
> 
> > On Wed, Jan 04, 2023 at 05:59:14PM +0100, Paul B Mahol wrote:
> > > Patches attached.
> >
> > >  af_aresample.c |    6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 94dacb46103e2bb9fbb6e1ca40675243d15069cd
> > 0003-avfilter-af_aresample-if-frame-parameters-change-upd.patch
> > > From 3959bcb707f52339bac41acc9aec856cad3aced1 Mon Sep 17 00:00:00 2001
> > > From: Paul B Mahol <onemda@gmail.com>
> > > Date: Wed, 4 Jan 2023 17:55:10 +0100
> > > Subject: [PATCH 3/3] avfilter/af_aresample: if frame parameters change
> > update
> > >  swr context
> > >
> > > Signed-off-by: Paul B Mahol <onemda@gmail.com>
> > > ---
> > >  libavfilter/af_aresample.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/libavfilter/af_aresample.c b/libavfilter/af_aresample.c
> > > index 7923377c8c..2744388f75 100644
> > > --- a/libavfilter/af_aresample.c
> > > +++ b/libavfilter/af_aresample.c
> > > @@ -209,8 +209,14 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > >      } else {
> > >          outsamplesref->pts  = AV_NOPTS_VALUE;
> > >      }
> > > +again:
> > >      ret = swr_convert_frame(aresample->swr, outsamplesref,
> > >                              (void *)insamplesref);
> > > +    if (ret & (AVERROR_INPUT_CHANGED | AVERROR_OUTPUT_CHANGED)) {
> > > +        swr_close(aresample->swr);
> > > +        goto again;
> > > +    }
> > > +
> > >      if (ret < 0) {
> > >          av_frame_free(&outsamplesref);
> > >          av_frame_free(&insamplesref);
> >
> > Are you sure this is not missing some flushing of internal samples ?
> >
> 
> Nope, point is not to flush all internal samples at all, just enough so
> clicks do not happen.

If there is 123ms audio in then there should be 123ms audio out
That is for the whole file. individual frames can differ due to buffering.
But if the buffers are discarded then i would expect that overall samples
are lost. This can cause problems with AV sync and timestamps too


> Clicks happen anyway so need to check why.

[...]
diff mbox series

Patch

From 0415ed37bee0c2b640920edad87ec927dda95fb5 Mon Sep 17 00:00:00 2001
From: Paul B Mahol <onemda@gmail.com>
Date: Wed, 4 Jan 2023 17:53:01 +0100
Subject: [PATCH 2/3] swresample/swresample_frame: fix regression in detecting
 changes

Do not overwrite return variable values, instead use different
one for checking results.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libswresample/swresample_frame.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/libswresample/swresample_frame.c b/libswresample/swresample_frame.c
index 53ac487136..319ce045a1 100644
--- a/libswresample/swresample_frame.c
+++ b/libswresample/swresample_frame.c
@@ -84,7 +84,7 @@  static int config_changed(SwrContext *s,
                           const AVFrame *out, const AVFrame *in)
 {
     AVChannelLayout ch_layout = { 0 };
-    int ret = 0;
+    int ret = 0, iret;
 
     if (in) {
 #if FF_API_OLD_CHANNEL_LAYOUT
@@ -96,8 +96,8 @@  FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
         } else
 #endif
-        if ((ret = av_channel_layout_copy(&ch_layout, &in->ch_layout)) < 0)
-            return ret;
+        if ((iret = av_channel_layout_copy(&ch_layout, &in->ch_layout)) < 0)
+            return iret;
         if (av_channel_layout_compare(&s->in_ch_layout, &ch_layout) ||
             s->in_sample_rate != in->sample_rate ||
             s->in_sample_fmt  != in->format) {
@@ -116,8 +116,8 @@  FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
         } else
 #endif
-        if ((ret = av_channel_layout_copy(&ch_layout, &out->ch_layout)) < 0)
-            return ret;
+        if ((iret = av_channel_layout_copy(&ch_layout, &out->ch_layout)) < 0)
+            return iret;
         if (av_channel_layout_compare(&s->out_ch_layout, &ch_layout) ||
             s->out_sample_rate != out->sample_rate ||
             s->out_sample_fmt  != out->format) {
-- 
2.37.2