[MPlayer-dev-eng] [PATCH] current pl_surround in mplayer CVS breaks playlist
D Richard Felker III
dalias at aerifal.cx
Sun Apr 7 02:24:55 CEST 2002
what's with this off-by-one error in sound? i've seen a few suggested
fixes, but how about just something like length &= ~1? :)
of course i may not know what i'm talking about...just a thought.
seems more likely to work than the "add 0.5 and round" trick though...
rich
On Sat, Apr 06, 2002 at 10:55:19PM +0200, Kis Gergely wrote:
> 2002-04-06, szo keltezéssel Felix Buenemann ezt írta:
> > On Saturday 06 April 2002 15:41, Kis Gergely wrote:
> > > Actually it tries to free() an already freed buffer in
> > > pl_surround.control(). The attached patch solves the problem.
> > thx, commited.
> >
> > Btw. is the extrasurround stuff now stable and ready to be commited? What is
> > the last patch for it?
> Well, I currently fight with ao_plugin.c
>
> I attach three patches.
>
> The first patch fixes a bug in ao_plugin.c. ao_plugin_local_data.len was
> not set to 0 on init and on uninit. This caused problems when using
> playlists.
>
> The second patch is the newest extrasurround patch (pl_surround.c).
> Changes:
> ->updated to current pl_surround.c
> ->subwoofer cutoff freq is set to 300 Hz. (After consulting with a more
> competent friend of mine.)
>
> If you compile mplayer, and try it out, it won't work. I tried to
> compile both in debug and normal mode, with gcc 2.95.4 (Debian
> prerelease) and gcc 3.0.4
>
> The problem is the previously described "real databuf len" - 1 bug.
>
> There is a workaround for that (look for Anders' mail).
>
> BUT:
> If you now apply the third patch, which only adds some fprintfs for
> debug purposes and recompile, it works!!!
>
> Now that's an X-file for me. I must be lame, or just found a compiler
> bug in both 2.95.4 and 3.0.4... I think the first version is more
> likely...:-(
>
> Could somebody please enlighten me?
> Or at least reproduce?
>
>
> Thanks,
> kisg
>
>
> --- ao_plugin.c.orig Fri Apr 5 10:16:13 2002
> +++ ao_plugin.c Sat Apr 6 21:21:31 2002
> @@ -172,6 +172,7 @@
> if(ao_plugin_local_data.buf)
> free(ao_plugin_local_data.buf);
> ao_plugin_local_data.buf=malloc(MAX_OUTBURST);
> + ao_plugin_local_data.len=0;
>
> if(!ao_plugin_local_data.buf) return 0;
>
> @@ -190,6 +191,7 @@
> if(ao_plugin_local_data.buf)
> free(ao_plugin_local_data.buf);
> ao_plugin_local_data.buf=NULL;
> + ao_plugin_local_data.len=0;
> }
>
> // stop playing and empty buffers (for seeking/pause)
> --- pl_surround.c.orig Sat Apr 6 19:10:52 2002
> +++ pl_surround.c Sat Apr 6 22:53:17 2002
> @@ -64,6 +64,7 @@
> int passthrough; // Just be a "NO-OP"
> int msecs; // Rear channel delay in milliseconds
> int16_t* databuf; // Output audio buffer
> + int databuf_len; // Output audio buffer length in samples
> int16_t* Ls_delaybuf; // circular buffer to be used for delaying Ls audio
> int16_t* Rs_delaybuf; // circular buffer to be used for delaying Rs audio
> int delaybuf_len; // delaybuf buffer length in samples
> @@ -72,10 +73,16 @@
> int rate; // input data rate
> int format; // input format
> int input_channels; // input channels
> + int output_channels; // output channels
> + float lowp_cutoff; // cutoff freq for the lowpass filter
> + double lowp_A, lowp_B; // parameters for lowpass filter
> + double lowp_outm1; // output for lowpass filter (??)
>
> } pl_surround_t;
>
> -static pl_surround_t pl_surround={0,20,NULL,NULL,NULL,0,0,NULL,0,0,0};
> +static pl_surround_t pl_surround={0,20,NULL,0,NULL,NULL,0,0,NULL,0,0,0,0,0,0,0,0};
> +
> +extern int audio_output_channels;
>
> // to set/get/query special features/parameters
> static int control(int cmd,int arg){
> @@ -89,9 +96,20 @@
> free(pl_surround.databuf); pl_surround.databuf = NULL;
> }
> // Allocate output buffer
> - pl_surround.databuf = calloc(ao_plugin_data.len, 1);
> +
> + fprintf(stderr, "pl_surround: We can produce: audio_plugin_data.len = %d\n",ao_plugin_data.len);
> +
> + pl_surround.databuf_len = ao_plugin_data.len / sizeof(int16_t) / pl_surround.output_channels;
> +
> + fprintf(stderr, "pl_surround: This means pl_surround.databuf_len = %d\n",pl_surround.databuf_len);
> +
> + pl_surround.databuf = calloc(pl_surround.databuf_len*pl_surround.output_channels, sizeof(int16_t));
> // Return back smaller len so we don't get overflowed...
> - ao_plugin_data.len /= 2;
> + // ao_plugin_data.len /= 2;
> + ao_plugin_data.len = pl_surround.databuf_len * sizeof(int16_t) * pl_surround.input_channels;
> +
> + fprintf(stderr, "pl_surround: We can receive: audio_plugin_data.len = %d\n",ao_plugin_data.len);
> +
> return CONTROL_OK;
> }
> return -1;
> @@ -113,6 +131,12 @@
> return 1;
> }
>
> + if (audio_output_channels != 4 && audio_output_channels != 6) {
> + fprintf(stderr, "pl_surround: I'm dumb and can only output 4 or 6 channel sound, using passtrough mode\n");
> + pl_surround.passthrough = 1;
> + return 1;
> + }
> +
> pl_surround.passthrough = 0;
>
> /* Store info on input format to expect */
> @@ -120,9 +144,16 @@
> pl_surround.format=ao_plugin_data.format;
> pl_surround.input_channels=ao_plugin_data.channels;
>
> + /* Store info on output channel number */
> + pl_surround.output_channels = audio_output_channels;
> +
> +
> // Input 2 channels, output will be 4 - tell ao_plugin
> - ao_plugin_data.channels = 4;
> - ao_plugin_data.sz_mult /= 2;
> +// ao_plugin_data.channels = 4;
> +// ao_plugin_data.sz_mult /= 2;
> +
> + ao_plugin_data.channels = pl_surround.output_channels;
> + ao_plugin_data.sz_mult /= pl_surround.output_channels / pl_surround.input_channels;
>
> // Figure out buffer space (in int16_ts) needed for the 15msec delay
> // Extra 31 samples allow for lowpass filter delay (taps-1)
> @@ -137,6 +168,16 @@
> pl_surround.filter_coefs_surround = calc_coefficients_7kHz_lowpass(pl_surround.rate);
> //dump_filter_coefficients(pl_surround.filter_coefs_surround);
> //testfilter(pl_surround.filter_coefs_surround, 32, pl_surround.rate);
> +
> + pl_surround.lowp_cutoff = 300;
> + if (pl_surround.lowp_cutoff > pl_surround.rate / 2) {
> + // Cutoff rate must be < sample rate / 2 (Nyquist rate)
> + pl_surround.lowp_cutoff = pl_surround.rate / 2 - 1;
> + }
> + pl_surround.lowp_B = exp ((-2.0 * M_PI * (pl_surround.lowp_cutoff / pl_surround.rate)));
> + pl_surround.lowp_A = 1 - pl_surround.lowp_B;
> + pl_surround.lowp_outm1 = 0.0;
> +
> return 1;
> }
>
> @@ -163,6 +204,7 @@
> pl_surround.delaybuf_pos = 0;
> memset(pl_surround.Ls_delaybuf, 0, sizeof(int16_t)*pl_surround.delaybuf_len);
> memset(pl_surround.Rs_delaybuf, 0, sizeof(int16_t)*pl_surround.delaybuf_len);
> + pl_surround.lowp_outm1 = 0;
> }
>
> // The beginnings of an active matrix...
> @@ -183,11 +225,11 @@
>
> if (pl_surround.passthrough) return 1;
>
> - // fprintf(stderr, "pl_surround: play %d bytes, %d samples\n", ao_plugin_data.len, samples);
>
> samples = ao_plugin_data.len / sizeof(int16_t) / pl_surround.input_channels;
> out = pl_surround.databuf; in = (int16_t *)ao_plugin_data.data;
>
> + fprintf(stderr, "pl_surround: play %d bytes, %d samples\n", ao_plugin_data.len, samples);
> // Testing - place a 1kHz tone on Lt and Rt in anti-phase: should decode in S
> //sinewave(in, samples, pl_surround.input_channels, 1000, 0.0, pl_surround.rate);
> //sinewave(&in[1], samples, pl_surround.input_channels, 1000, PI, pl_surround.rate);
> @@ -224,6 +266,26 @@
> #else
> out[3] = -out[2];
> #endif
> + if (pl_surround.output_channels == 6) {
> +
> + int32_t avg;
> + double d;
> + avg = (out[0] + out[1]) / 2;
> +
> + d = pl_surround.lowp_A * avg + pl_surround.lowp_B * pl_surround.lowp_outm1;
> +
> + if (d > 32767L) {
> + d = 32767L;
> + }
> + if (d < -32768L) {
> + d = -32768L;
> + }
> + pl_surround.lowp_outm1 = d;
> + out[5] = d;
> + // Only 4.1 output, center speaker remains silent
> + out[4] = 0;
> + }
> +
> // calculate and save surround for 20msecs time
> #ifdef SPLITREAR
> pl_surround.Ls_delaybuf[pl_surround.delaybuf_pos] =
> @@ -237,7 +299,7 @@
> pl_surround.delaybuf_pos %= pl_surround.delaybuf_len;
>
> // next samples...
> - in = &in[pl_surround.input_channels]; out = &out[4];
> + in = &in[pl_surround.input_channels]; out = &out[pl_surround.output_channels];
> }
>
> // Show some state
> @@ -245,6 +307,6 @@
>
> // Set output block/len
> ao_plugin_data.data=pl_surround.databuf;
> - ao_plugin_data.len=samples*sizeof(int16_t)*4;
> + ao_plugin_data.len=samples*sizeof(int16_t)*pl_surround.output_channels;
> return 1;
> }
> --- ao_plugin.c.bugfix1 Sat Apr 6 21:21:31 2002
> +++ ao_plugin.c Sat Apr 6 22:24:17 2002
> @@ -216,10 +216,17 @@
> // return: how many bytes can be played without blocking
> static int get_space(){
> double sz=(double)(driver()->get_space());
> - if(sz+(double)ao_plugin_local_data.len > (double)MAX_OUTBURST)
> + fprintf(stderr,"ao_plugin: get_space(): Output driver says %f free space\n",sz);
> + fprintf(stderr,"ao_plugin: get_space(): ao_plugin_local_data.len says %f\n",(double)ao_plugin_local_data.len);
> + if(sz+(double)ao_plugin_local_data.len > (double)MAX_OUTBURST) {
> + fprintf(stderr,"ao_plugin: get_space(): sz+localbuffer length is > MAX_OUTBURST\n");
> sz=(double)MAX_OUTBURST-(double)ao_plugin_local_data.len;
> + fprintf(stderr,"ao_plugin: get_space(): sz=MAX_OUTBURST - localdata.len = %f\n",sz);
> + }
> sz*=ao_plugin_data.sz_mult;
> + fprintf(stderr,"ao_plugin: get_space(): sz*sz_mult = %f\n",sz);
> sz+=ao_plugin_data.sz_fix;
> + fprintf(stderr,"ao_plugin: get_space(): sz+sz_fix = %f\n",sz);
> return (int)(sz);
> }
>
> @@ -230,6 +237,7 @@
> // Limit length to avoid over flow in plugins
> int tmp = get_space();
> int ret_len =(tmp<len)?tmp:len;
> + fprintf(stderr,"ao_plugin: play(): len = %d\n",len);
> if(ret_len){
> // Filter data
> ao_plugin_data.len=ret_len;
More information about the MPlayer-dev-eng
mailing list