diff mbox

[FFmpeg-devel] swresample/resample: add force_comp argument

Message ID CAJsO9v7=UULNWpiPqrDYHV+RGdbtwLxKVcPHZP1yVDWW2LyEsw@mail.gmail.com
State New
Headers show

Commit Message

Muhammad Faiz Nov. 20, 2016, 7:21 p.m. UTC
On 11/21/16, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Sun, Nov 20, 2016 at 03:56:28PM +0700, Muhammad Faiz wrote:
>> this allow to use phase_count_compensation on init (rather than
>> rebuild on swr_set_compensation) when options suggest that
>> soft compensation is enabled
>>
>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>> ---
>>  libswresample/resample.c            | 4 +++-
>>  libswresample/soxr_resample.c       | 2 +-
>>  libswresample/swresample.c          | 5 ++++-
>>  libswresample/swresample_internal.h | 3 ++-
>>  4 files changed, 10 insertions(+), 4 deletions(-)
>
> please document this in the code with a comment or 2
> its not immedeatly obvious from reading the code

ok, new patch attached

thanks

Comments

Michael Niedermayer Nov. 23, 2016, 10:59 a.m. UTC | #1
On Mon, Nov 21, 2016 at 02:21:07AM +0700, Muhammad Faiz wrote:
> On 11/21/16, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Sun, Nov 20, 2016 at 03:56:28PM +0700, Muhammad Faiz wrote:
> >> this allow to use phase_count_compensation on init (rather than
> >> rebuild on swr_set_compensation) when options suggest that
> >> soft compensation is enabled
> >>
> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
> >> ---
> >>  libswresample/resample.c            | 4 +++-
> >>  libswresample/soxr_resample.c       | 2 +-
> >>  libswresample/swresample.c          | 5 ++++-
> >>  libswresample/swresample_internal.h | 3 ++-
> >>  4 files changed, 10 insertions(+), 4 deletions(-)
> >
> > please document this in the code with a comment or 2
> > its not immedeatly obvious from reading the code
> 
> ok, new patch attached
> 
> thanks

>  resample.c            |    7 ++++++-
>  soxr_resample.c       |    3 ++-
>  swresample.c          |    7 ++++++-
>  swresample_internal.h |    3 ++-
>  4 files changed, 16 insertions(+), 4 deletions(-)
> ecc69d7aba15455780250ea23b661ebf136656c2  0001-swresample-resample-add-soft_compensation-argument.patch
> From 5846ba0a6df69e41783ab4caa88d392abd339ed1 Mon Sep 17 00:00:00 2001
> From: Muhammad Faiz <mfcc64@gmail.com>
> Date: Mon, 21 Nov 2016 02:08:40 +0700
> Subject: [PATCH] swresample/resample: add soft_compensation argument
> 
> this allow to use larger phase_count on init (rather than
> rebuild on swr_set_compensation) when options suggest that
> soft compensation is enabled
> 
> this affects only when exact_rational is enabled

what if an application reinitializes the context more often than
triggering soft compensation ?

maybe keeping statistics on actual soft_compensation use vs reinit
would be a better deciding factor
but maybe iam missing something

[...]
Muhammad Faiz Nov. 23, 2016, 5:21 p.m. UTC | #2
On 11/23/16, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Mon, Nov 21, 2016 at 02:21:07AM +0700, Muhammad Faiz wrote:
>> On 11/21/16, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> > On Sun, Nov 20, 2016 at 03:56:28PM +0700, Muhammad Faiz wrote:
>> >> this allow to use phase_count_compensation on init (rather than
>> >> rebuild on swr_set_compensation) when options suggest that
>> >> soft compensation is enabled
>> >>
>> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>> >> ---
>> >>  libswresample/resample.c            | 4 +++-
>> >>  libswresample/soxr_resample.c       | 2 +-
>> >>  libswresample/swresample.c          | 5 ++++-
>> >>  libswresample/swresample_internal.h | 3 ++-
>> >>  4 files changed, 10 insertions(+), 4 deletions(-)
>> >
>> > please document this in the code with a comment or 2
>> > its not immedeatly obvious from reading the code
>>
>> ok, new patch attached
>>
>> thanks
>
>>  resample.c            |    7 ++++++-
>>  soxr_resample.c       |    3 ++-
>>  swresample.c          |    7 ++++++-
>>  swresample_internal.h |    3 ++-
>>  4 files changed, 16 insertions(+), 4 deletions(-)
>> ecc69d7aba15455780250ea23b661ebf136656c2
>> 0001-swresample-resample-add-soft_compensation-argument.patch
>> From 5846ba0a6df69e41783ab4caa88d392abd339ed1 Mon Sep 17 00:00:00 2001
>> From: Muhammad Faiz <mfcc64@gmail.com>
>> Date: Mon, 21 Nov 2016 02:08:40 +0700
>> Subject: [PATCH] swresample/resample: add soft_compensation argument
>>
>> this allow to use larger phase_count on init (rather than
>> rebuild on swr_set_compensation) when options suggest that
>> soft compensation is enabled
>>
>> this affects only when exact_rational is enabled
>
> what if an application reinitializes the context more often than
> triggering soft compensation ?

In this case (or in worse condition such as no actual soft compensation happens
because timestamps are correct), the drawback is slower init/reinit, slightly
slower resampling and increased memory usage.

>
> maybe keeping statistics on actual soft_compensation use vs reinit
> would be a better deciding factor
> but maybe iam missing something

How I keep the statistics?

The main goal is to avoid rebuilding filter bank at runtime that
hurts real time application that use soft compensation e.g.
AV sync that uses non audio clock.

Actually I'm not sure with this.

Thank's
Muhammad Faiz Nov. 23, 2016, 6:15 p.m. UTC | #3
On 11/24/16, Muhammad Faiz <mfcc64@gmail.com> wrote:
> On 11/23/16, Michael Niedermayer <michael@niedermayer.cc> wrote:
>> On Mon, Nov 21, 2016 at 02:21:07AM +0700, Muhammad Faiz wrote:
>>> On 11/21/16, Michael Niedermayer <michael@niedermayer.cc> wrote:
>>> > On Sun, Nov 20, 2016 at 03:56:28PM +0700, Muhammad Faiz wrote:
>>> >> this allow to use phase_count_compensation on init (rather than
>>> >> rebuild on swr_set_compensation) when options suggest that
>>> >> soft compensation is enabled
>>> >>
>>> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>>> >> ---
>>> >>  libswresample/resample.c            | 4 +++-
>>> >>  libswresample/soxr_resample.c       | 2 +-
>>> >>  libswresample/swresample.c          | 5 ++++-
>>> >>  libswresample/swresample_internal.h | 3 ++-
>>> >>  4 files changed, 10 insertions(+), 4 deletions(-)
>>> >
>>> > please document this in the code with a comment or 2
>>> > its not immedeatly obvious from reading the code
>>>
>>> ok, new patch attached
>>>
>>> thanks
>>
>>>  resample.c            |    7 ++++++-
>>>  soxr_resample.c       |    3 ++-
>>>  swresample.c          |    7 ++++++-
>>>  swresample_internal.h |    3 ++-
>>>  4 files changed, 16 insertions(+), 4 deletions(-)
>>> ecc69d7aba15455780250ea23b661ebf136656c2
>>> 0001-swresample-resample-add-soft_compensation-argument.patch
>>> From 5846ba0a6df69e41783ab4caa88d392abd339ed1 Mon Sep 17 00:00:00 2001
>>> From: Muhammad Faiz <mfcc64@gmail.com>
>>> Date: Mon, 21 Nov 2016 02:08:40 +0700
>>> Subject: [PATCH] swresample/resample: add soft_compensation argument
>>>
>>> this allow to use larger phase_count on init (rather than
>>> rebuild on swr_set_compensation) when options suggest that
>>> soft compensation is enabled
>>>
>>> this affects only when exact_rational is enabled
>>
>> what if an application reinitializes the context more often than
>> triggering soft compensation ?
>
> In this case (or in worse condition such as no actual soft compensation
> happens
> because timestamps are correct), the drawback is slower init/reinit,
> slightly
> slower resampling and increased memory usage.
>
>>
>> maybe keeping statistics on actual soft_compensation use vs reinit
>> would be a better deciding factor
>> but maybe iam missing something
>
> How I keep the statistics?
>
> The main goal is to avoid rebuilding filter bank at runtime that
> hurts real time application that use soft compensation e.g.
> AV sync that uses non audio clock.
>
> Actually I'm not sure with this.
>
> Thank's
>

Probably explicit option is better.
I drop this.
Probably I will post a patch with more functionality.

Thank's
diff mbox

Patch

From 5846ba0a6df69e41783ab4caa88d392abd339ed1 Mon Sep 17 00:00:00 2001
From: Muhammad Faiz <mfcc64@gmail.com>
Date: Mon, 21 Nov 2016 02:08:40 +0700
Subject: [PATCH] swresample/resample: add soft_compensation argument

this allow to use larger phase_count on init (rather than
rebuild on swr_set_compensation) when options suggest that
soft compensation is enabled

this affects only when exact_rational is enabled

Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
---
 libswresample/resample.c            | 7 ++++++-
 libswresample/soxr_resample.c       | 3 ++-
 libswresample/swresample.c          | 7 ++++++-
 libswresample/swresample_internal.h | 3 ++-
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/libswresample/resample.c b/libswresample/resample.c
index 8635bf1..d1822e2 100644
--- a/libswresample/resample.c
+++ b/libswresample/resample.c
@@ -302,7 +302,7 @@  fail:
 
 static ResampleContext *resample_init(ResampleContext *c, int out_rate, int in_rate, int filter_size, int phase_shift, int linear,
                                     double cutoff0, enum AVSampleFormat format, enum SwrFilterType filter_type, double kaiser_beta,
-                                    double precision, int cheby, int exact_rational)
+                                    double precision, int cheby, int exact_rational, int soft_compensation)
 {
     double cutoff = cutoff0? cutoff0 : 0.97;
     double factor= FFMIN(out_rate * cutoff / in_rate, 1.0);
@@ -316,6 +316,10 @@  static ResampleContext *resample_init(ResampleContext *c, int out_rate, int in_r
         if (phase_count_exact <= phase_count) {
             phase_count_compensation = phase_count_exact * (phase_count / phase_count_exact);
             phase_count = phase_count_exact;
+            /* use larger phase_count when soft_compensation is true */
+            /* so rebuilding filter bank at runtime is not required */
+            if (soft_compensation)
+                phase_count = phase_count_compensation;
         }
     }
 
@@ -449,6 +453,7 @@  static int rebuild_filter_bank_with_compensation(ResampleContext *c)
 static int set_compensation(ResampleContext *c, int sample_delta, int compensation_distance){
     int ret;
 
+    /* handle users who call swr_set_compensation() manually without setting soft compensation options */
     if (compensation_distance) {
         ret = rebuild_filter_bank_with_compensation(c);
         if (ret < 0)
diff --git a/libswresample/soxr_resample.c b/libswresample/soxr_resample.c
index b9c6735..595cb64 100644
--- a/libswresample/soxr_resample.c
+++ b/libswresample/soxr_resample.c
@@ -30,7 +30,8 @@ 
 #include <soxr.h>
 
 static struct ResampleContext *create(struct ResampleContext *c, int out_rate, int in_rate, int filter_size, int phase_shift, int linear,
-        double cutoff, enum AVSampleFormat format, enum SwrFilterType filter_type, double kaiser_beta, double precision, int cheby, int exact_rational){
+        double cutoff, enum AVSampleFormat format, enum SwrFilterType filter_type, double kaiser_beta, double precision, int cheby,
+        int exact_rational, int soft_compensation){
     soxr_error_t error;
 
     soxr_datatype_t type =
diff --git a/libswresample/swresample.c b/libswresample/swresample.c
index 0ef4dea..8f85993 100644
--- a/libswresample/swresample.c
+++ b/libswresample/swresample.c
@@ -265,7 +265,12 @@  av_cold int swr_init(struct SwrContext *s){
     }
 
     if (s->out_sample_rate!=s->in_sample_rate || (s->flags & SWR_FLAG_RESAMPLE)){
-        s->resample = s->resampler->init(s->resample, s->out_sample_rate, s->in_sample_rate, s->filter_size, s->phase_shift, s->linear_interp, s->cutoff, s->int_sample_fmt, s->filter_type, s->kaiser_beta, s->precision, s->cheby, s->exact_rational);
+        /* suggested from swr_next_pts() */
+        /* if soft_compensation is false, swr_next_pts() will not call swr_set_compensation() */
+        int soft_compensation = s->min_compensation < FLT_MAX && s->soft_compensation_duration && s->max_soft_compensation;
+        s->resample = s->resampler->init(s->resample, s->out_sample_rate, s->in_sample_rate, s->filter_size,
+                                         s->phase_shift, s->linear_interp, s->cutoff, s->int_sample_fmt, s->filter_type,
+                                         s->kaiser_beta, s->precision, s->cheby, s->exact_rational, soft_compensation);
         if (!s->resample) {
             av_log(s, AV_LOG_ERROR, "Failed to initialize resampler\n");
             return AVERROR(ENOMEM);
diff --git a/libswresample/swresample_internal.h b/libswresample/swresample_internal.h
index f2ea5a2..8de8f1e 100644
--- a/libswresample/swresample_internal.h
+++ b/libswresample/swresample_internal.h
@@ -69,7 +69,8 @@  struct DitherContext {
 };
 
 typedef struct ResampleContext * (* resample_init_func)(struct ResampleContext *c, int out_rate, int in_rate, int filter_size, int phase_shift, int linear,
-                                    double cutoff, enum AVSampleFormat format, enum SwrFilterType filter_type, double kaiser_beta, double precision, int cheby, int exact_rational);
+                                    double cutoff, enum AVSampleFormat format, enum SwrFilterType filter_type, double kaiser_beta, double precision, int cheby,
+                                    int exact_rational, int soft_compensation);
 typedef void    (* resample_free_func)(struct ResampleContext **c);
 typedef int     (* multiple_resample_func)(struct ResampleContext *c, AudioData *dst, int dst_size, AudioData *src, int src_size, int *consumed);
 typedef int     (* resample_flush_func)(struct SwrContext *c);
-- 
2.5.0