| Message ID | 1481613363-26810-1-git-send-email-mfcc64@gmail.com |
|---|---|
| State | Accepted |
| Commit | 6a8c0d83572deabc4cd1920b6d71cc65a37acc57 |
| Headers | show |
On Tue, Dec 13, 2016 at 02:16:03PM +0700, Muhammad Faiz wrote: > This should fix Ticket6012 > > Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > --- > libswresample/resample.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libswresample/resample.c b/libswresample/resample.c > index 71dffb9..ce6a82f 100644 > --- a/libswresample/resample.c > +++ b/libswresample/resample.c > @@ -478,8 +478,9 @@ static int swri_resample(ResampleContext *c, > int64_t incr= (1LL<<32) * c->dst_incr / c->src_incr; > int new_size = (src_size * (int64_t)c->src_incr - frac + c->dst_incr - 1) / c->dst_incr; > > - dst_size= FFMIN(dst_size, new_size); > - c->dsp.resample_one(dst, src, dst_size, index2, incr); > + dst_size = FFMAX(FFMIN(dst_size, new_size), 0); > + if (dst_size > 0) > + c->dsp.resample_one(dst, src, dst_size, index2, incr); > > index += dst_size * c->dst_incr_div; > index += (frac + dst_size * (int64_t)c->dst_incr_mod) / c->src_incr; > @@ -494,7 +495,7 @@ static int swri_resample(ResampleContext *c, > int64_t delta_frac = (end_index - c->index) * c->src_incr - c->frac; > int delta_n = (delta_frac + c->dst_incr - 1) / c->dst_incr; > > - dst_size = FFMIN(dst_size, delta_n); > + dst_size = FFMAX(FFMIN(dst_size, delta_n), 0); Shouldnt this and other things be done outside the loop that calls swri_resample() ? [...]
On 12/13/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Dec 13, 2016 at 02:16:03PM +0700, Muhammad Faiz wrote: >> This should fix Ticket6012 >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> --- >> libswresample/resample.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/libswresample/resample.c b/libswresample/resample.c >> index 71dffb9..ce6a82f 100644 >> --- a/libswresample/resample.c >> +++ b/libswresample/resample.c >> @@ -478,8 +478,9 @@ static int swri_resample(ResampleContext *c, >> int64_t incr= (1LL<<32) * c->dst_incr / c->src_incr; >> int new_size = (src_size * (int64_t)c->src_incr - frac + >> c->dst_incr - 1) / c->dst_incr; >> >> - dst_size= FFMIN(dst_size, new_size); >> - c->dsp.resample_one(dst, src, dst_size, index2, incr); >> + dst_size = FFMAX(FFMIN(dst_size, new_size), 0); >> + if (dst_size > 0) >> + c->dsp.resample_one(dst, src, dst_size, index2, incr); >> >> index += dst_size * c->dst_incr_div; >> index += (frac + dst_size * (int64_t)c->dst_incr_mod) / >> c->src_incr; >> @@ -494,7 +495,7 @@ static int swri_resample(ResampleContext *c, >> int64_t delta_frac = (end_index - c->index) * c->src_incr - >> c->frac; >> int delta_n = (delta_frac + c->dst_incr - 1) / c->dst_incr; >> >> - dst_size = FFMIN(dst_size, delta_n); >> + dst_size = FFMAX(FFMIN(dst_size, delta_n), 0); > > Shouldnt this and other things be done outside the loop that > calls swri_resample() ? For performance reason probably yes, even swri_resample should be integrated to multiple_resample to avoid redundant calculations. But this is bugfix patch, and swri_resample should return proper dst_size. I just follow current code flow. Patch that improve performance is outside the scope of this patch. Probably I will post that if this patch is accepted. Thank's
On Tue, Dec 13, 2016 at 11:15:30PM +0700, Muhammad Faiz wrote: > On 12/13/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Tue, Dec 13, 2016 at 02:16:03PM +0700, Muhammad Faiz wrote: > >> This should fix Ticket6012 > >> > >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> > >> --- > >> libswresample/resample.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/libswresample/resample.c b/libswresample/resample.c > >> index 71dffb9..ce6a82f 100644 > >> --- a/libswresample/resample.c > >> +++ b/libswresample/resample.c > >> @@ -478,8 +478,9 @@ static int swri_resample(ResampleContext *c, > >> int64_t incr= (1LL<<32) * c->dst_incr / c->src_incr; > >> int new_size = (src_size * (int64_t)c->src_incr - frac + > >> c->dst_incr - 1) / c->dst_incr; > >> > >> - dst_size= FFMIN(dst_size, new_size); > >> - c->dsp.resample_one(dst, src, dst_size, index2, incr); > >> + dst_size = FFMAX(FFMIN(dst_size, new_size), 0); > >> + if (dst_size > 0) > >> + c->dsp.resample_one(dst, src, dst_size, index2, incr); > >> > >> index += dst_size * c->dst_incr_div; > >> index += (frac + dst_size * (int64_t)c->dst_incr_mod) / > >> c->src_incr; > >> @@ -494,7 +495,7 @@ static int swri_resample(ResampleContext *c, > >> int64_t delta_frac = (end_index - c->index) * c->src_incr - > >> c->frac; > >> int delta_n = (delta_frac + c->dst_incr - 1) / c->dst_incr; > >> > >> - dst_size = FFMIN(dst_size, delta_n); > >> + dst_size = FFMAX(FFMIN(dst_size, delta_n), 0); > > > > Shouldnt this and other things be done outside the loop that > > calls swri_resample() ? > > For performance reason probably yes, even swri_resample should be integrated > to multiple_resample to avoid redundant calculations. > > But this is bugfix patch, and swri_resample should return proper dst_size. > I just follow current code flow. > > Patch that improve performance is outside the scope of this patch. Probably > I will post that if this patch is accepted. LGTM thx [...]
On 12/14/16, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Tue, Dec 13, 2016 at 11:15:30PM +0700, Muhammad Faiz wrote: >> On 12/13/16, Michael Niedermayer <michael@niedermayer.cc> wrote: >> > On Tue, Dec 13, 2016 at 02:16:03PM +0700, Muhammad Faiz wrote: >> >> This should fix Ticket6012 >> >> >> >> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> >> >> --- >> >> libswresample/resample.c | 7 ++++--- >> >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/libswresample/resample.c b/libswresample/resample.c >> >> index 71dffb9..ce6a82f 100644 >> >> --- a/libswresample/resample.c >> >> +++ b/libswresample/resample.c >> >> @@ -478,8 +478,9 @@ static int swri_resample(ResampleContext *c, >> >> int64_t incr= (1LL<<32) * c->dst_incr / c->src_incr; >> >> int new_size = (src_size * (int64_t)c->src_incr - frac + >> >> c->dst_incr - 1) / c->dst_incr; >> >> >> >> - dst_size= FFMIN(dst_size, new_size); >> >> - c->dsp.resample_one(dst, src, dst_size, index2, incr); >> >> + dst_size = FFMAX(FFMIN(dst_size, new_size), 0); >> >> + if (dst_size > 0) >> >> + c->dsp.resample_one(dst, src, dst_size, index2, incr); >> >> >> >> index += dst_size * c->dst_incr_div; >> >> index += (frac + dst_size * (int64_t)c->dst_incr_mod) / >> >> c->src_incr; >> >> @@ -494,7 +495,7 @@ static int swri_resample(ResampleContext *c, >> >> int64_t delta_frac = (end_index - c->index) * c->src_incr - >> >> c->frac; >> >> int delta_n = (delta_frac + c->dst_incr - 1) / c->dst_incr; >> >> >> >> - dst_size = FFMIN(dst_size, delta_n); >> >> + dst_size = FFMAX(FFMIN(dst_size, delta_n), 0); >> > >> > Shouldnt this and other things be done outside the loop that >> > calls swri_resample() ? >> >> For performance reason probably yes, even swri_resample should be >> integrated >> to multiple_resample to avoid redundant calculations. >> >> But this is bugfix patch, and swri_resample should return proper >> dst_size. >> I just follow current code flow. >> >> Patch that improve performance is outside the scope of this patch. >> Probably >> I will post that if this patch is accepted. > > LGTM > > thx Applied Thank's
diff --git a/libswresample/resample.c b/libswresample/resample.c index 71dffb9..ce6a82f 100644 --- a/libswresample/resample.c +++ b/libswresample/resample.c @@ -478,8 +478,9 @@ static int swri_resample(ResampleContext *c, int64_t incr= (1LL<<32) * c->dst_incr / c->src_incr; int new_size = (src_size * (int64_t)c->src_incr - frac + c->dst_incr - 1) / c->dst_incr; - dst_size= FFMIN(dst_size, new_size); - c->dsp.resample_one(dst, src, dst_size, index2, incr); + dst_size = FFMAX(FFMIN(dst_size, new_size), 0); + if (dst_size > 0) + c->dsp.resample_one(dst, src, dst_size, index2, incr); index += dst_size * c->dst_incr_div; index += (frac + dst_size * (int64_t)c->dst_incr_mod) / c->src_incr; @@ -494,7 +495,7 @@ static int swri_resample(ResampleContext *c, int64_t delta_frac = (end_index - c->index) * c->src_incr - c->frac; int delta_n = (delta_frac + c->dst_incr - 1) / c->dst_incr; - dst_size = FFMIN(dst_size, delta_n); + dst_size = FFMAX(FFMIN(dst_size, delta_n), 0); if (dst_size > 0) { /* resample_linear and resample_common should have same behavior * when frac and dst_incr_mod are zero */
This should fix Ticket6012 Signed-off-by: Muhammad Faiz <mfcc64@gmail.com> --- libswresample/resample.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)