Re: [FFmpeg-devel] [PATCH 1/2] libswresample: Avoid needlessly large on-stack array.

2014-09-06 Thread Reimar Döffinger 
On Wed, Sep 03, 2014 at 10:48:38PM +0200, Michael Niedermayer wrote:
> On Wed, Sep 03, 2014 at 09:40:55PM +0200, Reimar Döffinger wrote:
> > We only actually need to use a tiny part of it.
> > Unfortunately we seem to have no real test coverage on
> > the code, so this is a bit risky.
> > 
> > Signed-off-by: Reimar Döffinger 
> > ---
> >  libswresample/rematrix.c | 23 +--
> >  1 file changed, 17 insertions(+), 6 deletions(-)
> [...]
> > @@ -295,10 +296,10 @@ av_cold static int auto_matrix(SwrContext *s)
> >  av_assert0(0);
> >  }
> >  
> > -for(out_i=i=0; i<64; i++){
> > +for(out_i=i=0; i >  double sum=0;
> > -int in_i=0;
> > -for(j=0; j<64; j++){
> > +in_i=0;
> > +for(j=0; j >  s->matrix[out_i][in_i]= matrix[i][j];
> >  if(matrix[i][j]){
> >  sum += fabs(matrix[i][j]);
> > @@ -310,6 +311,16 @@ av_cold static int auto_matrix(SwrContext *s)
> >  if(out_ch_layout & (1ULL< >  out_i++;
> >  }
> > +for (; i < 64; i++) {
> > +if (in_ch_layout & out_ch_layout & (1ULL< > +s->matrix[out_i][in_i] = 1.0;
> > +maxcoef = FFMAX(maxcoef, 1.0);
> > +}
> > +if (in_ch_layout & (1ULL< > +in_i++;
> > +if (out_ch_layout & (1ULL< > +out_i++;
> > +}
> >  if(s->rematrix_volume  < 0)
> >  maxcoef = -s->rematrix_volume;
> 
> if (i and j are small enough)
> s->matrix[out_i][in_i]= matrix[i][j];
> else
> s->matrix[out_i][in_i]= 0
> sum += fabs(s->matrix[out_i][in_i]);
> 
> in the first loop should be simpler than a seperate loop

I am a bit undecided on whether the resulting code is really simpler,
but at least the diff is simpler.
I'll send a new patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libswresample: Avoid needlessly large on-stack array.

2014-09-03 Thread Michael Niedermayer 
On Wed, Sep 03, 2014 at 09:40:55PM +0200, Reimar Döffinger wrote:
> We only actually need to use a tiny part of it.
> Unfortunately we seem to have no real test coverage on
> the code, so this is a bit risky.
> 
> Signed-off-by: Reimar Döffinger 
> ---
>  libswresample/rematrix.c | 23 +--
>  1 file changed, 17 insertions(+), 6 deletions(-)
[...]
> @@ -295,10 +296,10 @@ av_cold static int auto_matrix(SwrContext *s)
>  av_assert0(0);
>  }
>  
> -for(out_i=i=0; i<64; i++){
> +for(out_i=i=0; i  double sum=0;
> -int in_i=0;
> -for(j=0; j<64; j++){
> +in_i=0;
> +for(j=0; j  s->matrix[out_i][in_i]= matrix[i][j];
>  if(matrix[i][j]){
>  sum += fabs(matrix[i][j]);
> @@ -310,6 +311,16 @@ av_cold static int auto_matrix(SwrContext *s)
>  if(out_ch_layout & (1ULL<  out_i++;
>  }
> +for (; i < 64; i++) {
> +if (in_ch_layout & out_ch_layout & (1ULL< +s->matrix[out_i][in_i] = 1.0;
> +maxcoef = FFMAX(maxcoef, 1.0);
> +}
> +if (in_ch_layout & (1ULL< +in_i++;
> +if (out_ch_layout & (1ULL< +out_i++;
> +}
>  if(s->rematrix_volume  < 0)
>  maxcoef = -s->rematrix_volume;

if (i and j are small enough)
s->matrix[out_i][in_i]= matrix[i][j];
else
s->matrix[out_i][in_i]= 0
sum += fabs(s->matrix[out_i][in_i]);

in the first loop should be simpler than a seperate loop

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 


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