Re: [FFmpeg-devel] [PATCH 1/2] swresample: fix phase_count calculation

2016-06-17 Thread Muhammad Faiz 
On Fri, Jun 17, 2016 at 2:23 PM, Michael Niedermayer
 wrote:
> On Fri, Jun 17, 2016 at 05:48:55AM +0700, Muhammad Faiz wrote:
>> On Thu, Jun 16, 2016 at 10:03 PM, Michael Niedermayer
>>  wrote:
>> > On Thu, Jun 16, 2016 at 12:31:03AM +0700, Muhammad Faiz wrote:
>> >> support odd phase_count
>> >> stick to low phase_count until set_compensation is called
>> >
>> > can you split these in 2 seperate patches ?
>> >
>> >
>> > [...]
>> >> @@ -382,6 +382,9 @@ static ResampleContext *resample_init(ResampleContext 
>> >> *c, int out_rate, int in_r
>> >>  c->index= -phase_count*((c->filter_length-1)/2);
>> >>  c->frac= 0;
>> >>
>> >> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
>> >> %d/%d)\n",
>> >> +   c->phase_count, c->dst_incr, c->dst_incr_div, 
>> >> c->dst_incr_mod, c->src_incr);*/
>> >> +
>> >>  swri_resample_dsp_init(c);
>> >>
>> >>  return c;
>> > [...]
>> >> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
>> >> %d/%d)\n",
>> >> +   c->phase_count, c->dst_incr, c->dst_incr_div, 
>> >> c->dst_incr_mod, c->src_incr);*/
>> >
>> > these disabled debug av_log() should probably not be pushed
>> >
>>
>> OK, patches attached
>>
>> Thank's
>
>>  resample.c |   17 -
>>  1 file changed, 8 insertions(+), 9 deletions(-)
>> 23ee69b87f2a83e8d8d394eabba6cf727940127b  
>> 0001-swresample-resample-add-support-for-odd-phase_count.patch
>> From 8cb201c678337dbf7c73545919e183f24ada2269 Mon Sep 17 00:00:00 2001
>> From: Muhammad Faiz 
>> Date: Fri, 17 Jun 2016 05:30:37 +0700
>> Subject: [PATCH 1/2] swresample/resample: add support for odd phase_count
>>
>> because exact_rational does not guarantee
>> that phase_count is even
>>
>> Signed-off-by: Muhammad Faiz 
>
> LGTM
>
applied

> [...]
>
>> diff --git a/libswresample/resample.h b/libswresample/resample.h
>> index 53788c4..2c29959 100644
>> --- a/libswresample/resample.h
>> +++ b/libswresample/resample.h
>> @@ -51,6 +51,7 @@ typedef struct ResampleContext {
>>  enum AVSampleFormat format;
>>  int felem_size;
>>  int filter_shift;
>
>> +int phase_count_comp;
>
> comp makes me think of computation before compensation, please
> use a different name and document the field
>
> should be ok otherwise
>
> thanks
>
renamed to phase_count_compensation
and applied

thanks
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] swresample: fix phase_count calculation

2016-06-17 Thread Michael Niedermayer 
On Fri, Jun 17, 2016 at 05:48:55AM +0700, Muhammad Faiz wrote:
> On Thu, Jun 16, 2016 at 10:03 PM, Michael Niedermayer
>  wrote:
> > On Thu, Jun 16, 2016 at 12:31:03AM +0700, Muhammad Faiz wrote:
> >> support odd phase_count
> >> stick to low phase_count until set_compensation is called
> >
> > can you split these in 2 seperate patches ?
> >
> >
> > [...]
> >> @@ -382,6 +382,9 @@ static ResampleContext *resample_init(ResampleContext 
> >> *c, int out_rate, int in_r
> >>  c->index= -phase_count*((c->filter_length-1)/2);
> >>  c->frac= 0;
> >>
> >> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
> >> %d/%d)\n",
> >> +   c->phase_count, c->dst_incr, c->dst_incr_div, c->dst_incr_mod, 
> >> c->src_incr);*/
> >> +
> >>  swri_resample_dsp_init(c);
> >>
> >>  return c;
> > [...]
> >> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
> >> %d/%d)\n",
> >> +   c->phase_count, c->dst_incr, c->dst_incr_div, c->dst_incr_mod, 
> >> c->src_incr);*/
> >
> > these disabled debug av_log() should probably not be pushed
> >
> 
> OK, patches attached
> 
> Thank's

>  resample.c |   17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 23ee69b87f2a83e8d8d394eabba6cf727940127b  
> 0001-swresample-resample-add-support-for-odd-phase_count.patch
> From 8cb201c678337dbf7c73545919e183f24ada2269 Mon Sep 17 00:00:00 2001
> From: Muhammad Faiz 
> Date: Fri, 17 Jun 2016 05:30:37 +0700
> Subject: [PATCH 1/2] swresample/resample: add support for odd phase_count
> 
> because exact_rational does not guarantee
> that phase_count is even
> 
> Signed-off-by: Muhammad Faiz 

LGTM

[...]

> diff --git a/libswresample/resample.h b/libswresample/resample.h
> index 53788c4..2c29959 100644
> --- a/libswresample/resample.h
> +++ b/libswresample/resample.h
> @@ -51,6 +51,7 @@ typedef struct ResampleContext {
>  enum AVSampleFormat format;
>  int felem_size;
>  int filter_shift;

> +int phase_count_comp;

comp makes me think of computation before compensation, please
use a different name and document the field

should be ok otherwise

thanks

[...]


-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] swresample: fix phase_count calculation

2016-06-16 Thread Muhammad Faiz 
On Thu, Jun 16, 2016 at 10:03 PM, Michael Niedermayer
 wrote:
> On Thu, Jun 16, 2016 at 12:31:03AM +0700, Muhammad Faiz wrote:
>> support odd phase_count
>> stick to low phase_count until set_compensation is called
>
> can you split these in 2 seperate patches ?
>
>
> [...]
>> @@ -382,6 +382,9 @@ static ResampleContext *resample_init(ResampleContext 
>> *c, int out_rate, int in_r
>>  c->index= -phase_count*((c->filter_length-1)/2);
>>  c->frac= 0;
>>
>> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
>> %d/%d)\n",
>> +   c->phase_count, c->dst_incr, c->dst_incr_div, c->dst_incr_mod, 
>> c->src_incr);*/
>> +
>>  swri_resample_dsp_init(c);
>>
>>  return c;
> [...]
>> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
>> %d/%d)\n",
>> +   c->phase_count, c->dst_incr, c->dst_incr_div, c->dst_incr_mod, 
>> c->src_incr);*/
>
> these disabled debug av_log() should probably not be pushed
>

OK, patches attached

Thank's
From 8cb201c678337dbf7c73545919e183f24ada2269 Mon Sep 17 00:00:00 2001
From: Muhammad Faiz 
Date: Fri, 17 Jun 2016 05:30:37 +0700
Subject: [PATCH 1/2] swresample/resample: add support for odd phase_count

because exact_rational does not guarantee
that phase_count is even

Signed-off-by: Muhammad Faiz 
---
 libswresample/resample.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/libswresample/resample.c b/libswresample/resample.c
index 1b1d83e..16c2a39 100644
--- a/libswresample/resample.c
+++ b/libswresample/resample.c
@@ -144,9 +144,10 @@ static double bessel(double x) {
 static int build_filter(ResampleContext *c, void *filter, double factor, int tap_count, int alloc, int phase_count, int scale,
 int filter_type, double kaiser_beta){
 int ph, i;
+int ph_nb = phase_count % 2 ? phase_count : phase_count / 2 + 1;
 double x, y, w, t, s;
 double *tab = av_malloc_array(tap_count+1,  sizeof(*tab));
-double *sin_lut = av_malloc_array(phase_count / 2 + 1, sizeof(*sin_lut));
+double *sin_lut = av_malloc_array(ph_nb, sizeof(*sin_lut));
 const int center= (tap_count-1)/2;
 
 if (!tab || !sin_lut)
@@ -156,13 +157,11 @@ static int build_filter(ResampleContext *c, void *filter, double factor, int tap
 if (factor > 1.0)
 factor = 1.0;
 
-av_assert0(phase_count == 1 || phase_count % 2 == 0);
-
 if (factor == 1.0) {
-for (ph = 0; ph <= phase_count / 2; ph++)
+for (ph = 0; ph < ph_nb; ph++)
 sin_lut[ph] = sin(M_PI * ph / phase_count);
 }
-for(ph = 0; ph <= phase_count / 2; ph++) {
+for(ph = 0; ph < ph_nb; ph++) {
 double norm = 0;
 s = sin_lut[ph];
 for(i=0;i<=tap_count;i++) {
@@ -203,6 +202,7 @@ static int build_filter(ResampleContext *c, void *filter, double factor, int tap
 case AV_SAMPLE_FMT_S16P:
 for(i=0;i

Re: [FFmpeg-devel] [PATCH 1/2] swresample: fix phase_count calculation

2016-06-16 Thread Michael Niedermayer 
On Thu, Jun 16, 2016 at 12:31:03AM +0700, Muhammad Faiz wrote:
> support odd phase_count
> stick to low phase_count until set_compensation is called

can you split these in 2 seperate patches ?


[...]
> @@ -382,6 +382,9 @@ static ResampleContext *resample_init(ResampleContext *c, 
> int out_rate, int in_r
>  c->index= -phase_count*((c->filter_length-1)/2);
>  c->frac= 0;
>  
> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
> %d/%d)\n",
> +   c->phase_count, c->dst_incr, c->dst_incr_div, c->dst_incr_mod, 
> c->src_incr);*/
> +
>  swri_resample_dsp_init(c);
>  
>  return c;
[...]
> +/*av_log(NULL, AV_LOG_ERROR, "phase_count = %d, dst_incr = %d (%d + 
> %d/%d)\n",
> +   c->phase_count, c->dst_incr, c->dst_incr_div, c->dst_incr_mod, 
> c->src_incr);*/

these disabled debug av_log() should probably not be pushed

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/2] swresample: fix phase_count calculation

2016-06-15 Thread Muhammad Faiz 
support odd phase_count
stick to low phase_count until set_compensation is called

Signed-off-by: Muhammad Faiz 
---
 libswresample/resample.c | 83 +---
 libswresample/resample.h |  1 +
 2 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/libswresample/resample.c b/libswresample/resample.c
index 1b1d83e..3b01408 100644
--- a/libswresample/resample.c
+++ b/libswresample/resample.c
@@ -144,9 +144,10 @@ static double bessel(double x) {
 static int build_filter(ResampleContext *c, void *filter, double factor, int 
tap_count, int alloc, int phase_count, int scale,
 int filter_type, double kaiser_beta){
 int ph, i;
+int ph_nb = phase_count % 2 ? phase_count : phase_count / 2 + 1;
 double x, y, w, t, s;
 double *tab = av_malloc_array(tap_count+1,  sizeof(*tab));
-double *sin_lut = av_malloc_array(phase_count / 2 + 1, sizeof(*sin_lut));
+double *sin_lut = av_malloc_array(ph_nb, sizeof(*sin_lut));
 const int center= (tap_count-1)/2;
 
 if (!tab || !sin_lut)
@@ -156,13 +157,11 @@ static int build_filter(ResampleContext *c, void *filter, 
double factor, int tap
 if (factor > 1.0)
 factor = 1.0;
 
-av_assert0(phase_count == 1 || phase_count % 2 == 0);
-
 if (factor == 1.0) {
-for (ph = 0; ph <= phase_count / 2; ph++)
+for (ph = 0; ph < ph_nb; ph++)
 sin_lut[ph] = sin(M_PI * ph / phase_count);
 }
-for(ph = 0; ph <= phase_count / 2; ph++) {
+for(ph = 0; ph < ph_nb; ph++) {
 double norm = 0;
 s = sin_lut[ph];
 for(i=0;i<=tap_count;i++) {
@@ -203,6 +202,7 @@ static int build_filter(ResampleContext *c, void *filter, 
double factor, int tap
 case AV_SAMPLE_FMT_S16P:
 for(i=0;i 1 && 
phase_count_exact < INT_MAX/2)
-phase_count_exact *= 2;
-
 if (phase_count_exact <= phase_count) {
-/* FIXME this is not required when soft compensation is disabled */
-phase_count_exact *= phase_count / phase_count_exact;
+phase_count_comp = phase_count_exact * (phase_count / 
phase_count_exact);
 phase_count = phase_count_exact;
 }
 }
@@ -360,6 +359,7 @@ static ResampleContext *resample_init(ResampleContext *c, 
int out_rate, int in_r
 c->filter_bank   = av_calloc(c->filter_alloc, 
(phase_count+1)*c->felem_size);