First try
Hi brad,
I just tried to move global variables to pcm_a2dp struct... However, I seem to experience a loss in quality. I'm really unsure about it. I don't understand why as it is a straight forward fix. Could you please review it? It's based on the latest cvs.
I also tried a "make dist", but I got a btsco-0.2.tar.gz, which makes me say configure.in is not up to date !
Thank you,
Frédéric
Index: pcm_a2dp.c
===================================================================
RCS file: /cvsroot/bluetooth-alsa/btsco/alsa-plugins/pcm_a2dp.c,v retrieving revision 1.9
diff -r1.9 pcm_a2dp.c
52,54d51
< time_t timestamp=0;
< uint16_t seq_num=1;
< int frame_count=0;
56,66d52
< static char bufe[BUFS];
< int lenbufe=0;
<
< unsigned long nbytes=0;
< unsigned long total_time=0;
< struct timeval t0;
< struct itimerval itimer;
< struct sigaction sa;
< struct timeval tsend;
< int fpr=0;
<
563a550,563
Quoted 8 lines
char bufe[BUFS]; int lenbufe;//=0;unsigned long nbytes;//=0;
Quoted 6 lines
struct timeval tsend;
time_t timestamp;//=0; uint16_t seq_num;//=1; int frame_count;//=0;
671,673c671,673
< if(lenbufe<codesize && lenbufe+datatoread<sizeof(bufe)){ // if not enough data in bufe to encode and there is space in bufe
< memcpy(bufe+lenbufe,buf,datatoread);// we read data to bufe
< lenbufe+=datatoread;
---
Quoted 3 lines
if(a2dp->lenbufe<codesize && a2dp->lenbufe+datatoread<sizeof(a2dp->bufe)){ // if not enough data in bufe to encode and there is space in bufe
memcpy(a2dp->bufe+a2dp->lenbufe,buf,datatoread);// we read data to bufe
a2dp->lenbufe+=datatoread;677,682c677,682
< if(lenbufe>=codesize && a2dp->len + a2dp->sbc.len < 678){ // if enough data in bufe to encode and not enough frame to fill up mtu: encoding
< change_endian(bufe,codesize); // changing the endianness
< len = sbc_encode(&(a2dp->sbc), bufe, codesize); //encode
< memmove(bufe, bufe + len, lenbufe - len); //shift the bufe
< lenbufe-=len;
< nbytes+=len;
---
Quoted 6 lines
if(a2dp->lenbufe>=codesize && a2dp->len + a2dp->sbc.len < 678){ // if enough data in bufe to encode and not enough frame to fill up mtu: encoding
change_endian(a2dp->bufe,codesize); // changing the endianness
len = sbc_encode(&(a2dp->sbc), a2dp->bufe, codesize); //encode
memmove(a2dp->bufe, a2dp->bufe + len, a2dp->lenbufe - len); //shift the bufe
a2dp->lenbufe-=len;
a2dp->nbytes+=len;686c686
< frame_count++;
---
Quoted 1 line
a2dp->frame_count++;
695c695
< dt.tv_usec=1000000*a2dp->sbc.subbands*a2dp->sbc.blocks*frame_count/io->rate; // time interval between transmitions
---
Quoted 1 line
dt.tv_usec=1000000*a2dp->sbc.subbands*a2dp->sbc.blocks*a2dp->frame_count/io->rate; // time interval between transmitions
697c697
< if(time_to_wait(&tsend, &dt)==0){ // time to send data
---
Quoted 1 line
if(time_to_wait(&a2dp->tsend, &dt)==0){ // time to send data700c700
< payload_header.frame_count=frame_count;
---
Quoted 1 line
payload_header.frame_count=a2dp->frame_count;
703,704c703,704
< packet_header.sequence_number = htons(seq_num);
< packet_header.timestamp = htonl(timestamp);
---
Quoted 2 lines
packet_header.sequence_number = htons(a2dp->seq_num); packet_header.timestamp = htonl(a2dp->timestamp);
706c706
< timestamp += (a2dp->sbc.blocks + 1)*4 * (a2dp->sbc.subbands + 1)*4;
---
Quoted 1 line
a2dp->timestamp += (a2dp->sbc.blocks + 1)*4 * (a2dp->sbc.subbands + 1)*4;
711c711
< frame_count=0;
---
Quoted 1 line
a2dp->frame_count=0;
713c713
< seq_num++;
---
Quoted 1 line
a2dp->seq_num++;
734d733
<
737a737,754
Quoted 10 lines
struct timeval timeofday;
gettimeofday(&timeofday, NULL);
/*
DBG("(%2d%3d): %d + %d * %d (=%d)",
(timeofday.tv_sec )%100, // s
(timeofday.tv_usec/1000)%1000, // ms
(int)areas->first, (int)areas->step, offset, (areas->first + areas->step * offset));
/*
DBG("(%2d%3d): %d * %d * %d * 2 (=%d) x %d",Quoted 8 lines
(timeofday.tv_sec )%100, // s (timeofday.tv_usec/1000)%1000, // ms a2dp->sbc.subbands, // 8 a2dp->sbc.blocks, // 16 a2dp->sbc.channels, // 2 codesize, // 512 datatoread); // 512 */
739,741c756,758
< if(lenbufe<codesize){
< memcpy(bufe+lenbufe,buf,datatoread);
< lenbufe+=datatoread;
---
Quoted 3 lines
if(a2dp->lenbufe<codesize){
memcpy(a2dp->bufe+a2dp->lenbufe,buf,datatoread);
a2dp->lenbufe+=datatoread;745,750c762,767
< if(lenbufe>=codesize){ //enough data to encode
< change_endian(bufe,codesize); // changing the endianness
< len = sbc_encode(&(a2dp->sbc), bufe, codesize); //encode
< memmove(bufe, bufe + len, lenbufe - len); //shift the bufe
< lenbufe-=len;
< nbytes+=len;
---
Quoted 6 lines
if(a2dp->lenbufe>=codesize){ //enough data to encode
change_endian(a2dp->bufe,codesize); // changing the endianness
len = sbc_encode(&(a2dp->sbc), a2dp->bufe, codesize); //encode
memmove(a2dp->bufe, a2dp->bufe + len, a2dp->lenbufe - len); //shift the bufe
a2dp->lenbufe-=len;
a2dp->nbytes+=len;756c773
< dt.tv_usec=1000000*a2dp->sbc.subbands*a2dp->sbc.blocks*frame_count/io->rate;
---
Quoted 1 line
dt.tv_usec=1000000*a2dp->sbc.subbands*a2dp->sbc.blocks*a2dp->frame_count/io->rate;
759c776
< payload_header.frame_count=frame_count;
---
Quoted 1 line
payload_header.frame_count=a2dp->frame_count;
762,763c779,780
< packet_header.sequence_number = htons(seq_num);
< packet_header.timestamp = htonl(timestamp);
---
Quoted 2 lines
packet_header.sequence_number = htons(a2dp->seq_num); packet_header.timestamp = htonl(a2dp->timestamp);
765c782
< timestamp += (a2dp->sbc.blocks + 1)*4 * (a2dp->sbc.subbands + 1)*4;
---
Quoted 1 line
a2dp->timestamp += (a2dp->sbc.blocks + 1)*4 * (a2dp->sbc.subbands + 1)*4;
768c785
< sleeptill(&tsend, &dt);
---
Quoted 1 line
sleeptill(&a2dp->tsend, &dt);
774c791
< frame_count=0;
---
Quoted 1 line
a2dp->frame_count=0;
776c793
< seq_num++;
---
Quoted 1 line
a2dp->seq_num++;
778c795
< frame_count++;
---
Quoted 1 line
a2dp->frame_count++;
974c991
< DBG("a2dp %p", a2dp);
---
Quoted 1 line
DBG("TEST a2dp %p", a2dp);1042a1060
Quoted 1 line
a2dp->seq_num = 1;
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Fr=E9d=E9ric
Quoted 5 lines
I just tried to move global variables to
pcm_a2dpstruct... However, I seem to experience a loss in quality. I'm really unsure about it. I don't understand why as it is a straight forward fix. Could you please review it?
It's based on the latest cvs.
could you resend as a 'diff -u' output?
thanks for your work on this...
brad
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Hi brad,
Quoted 3 lines
could you resend as a 'diff -u' output?
this is the command line I use is that correct?
cvs diff -u pcm_a2dp.c > pcm_a2dp.c.diff-u
Now the output has +- instead of ><
Frederic
Index: alsa-plugins/pcm_a2dp.c
===================================================================
RCS file: /cvsroot/bluetooth-alsa/btsco/alsa-plugins/pcm_a2dp.c,v retrieving revision 1.9
pcm_a2dp.c +70 -52
@@ -49,21 +49,7 @@#include "../a2dp.h" #define NONSPECAUDIO 1 -time_t timestamp=0 ; -uint16_t seq_num=1 ; -int frame_count=0 ;#define BUFS 1024 -static char bufe[BUFS ]; -int lenbufe=0 ; - -unsigned long nbytes=0 ; -unsigned long total_time=0 ; -struct timeval t0; -struct itimerval itimer; -struct sigaction sa; -struct timeval tsend; -int fpr=0 ; -#define min (X ,Y ) ((X) < (Y) ? (X) : (Y)) #define DBG(fmt, arg...) printf("DEBUG: %s: " fmt "\n" , __FUNCTION__ , ## arg)
@@ -561,6 +547,20 @@unsigned int len;unsigned int frame_bytes;int use_rfcomm; + +char bufe[BUFS ]; +int lenbufe;//=0; + +unsigned long nbytes;//=0; + + + +struct timeval tsend; + +time_t timestamp;//=0; +uint16_t seq_num;//=1; +int frame_count;//=0; + } snd_pcm_a2dp_t;static void inline a2dp_get (snd_pcm_a2dp_t *a2dp)
@@ -668,22 +668,22 @@ codesize=a2dp->sbc.subbands*a2dp->sbc.blocks*a2dp->sbc.channels*2 ;// size of data encoded by sbc_encode in one call datatoread=min (codesize,size*a2dp->frame_bytes);// amount of data to read buf = (char *) areas->addr + (areas->first + areas->step * offset) /8 ; -if (lenbufe<codesize && lenbufe+datatoread<sizeof (bufe)){// if not enough data in bufe to encode and there is space in bufe -memcpy (bufe+lenbufe,buf,datatoread);// we read data to bufe - lenbufe+=datatoread; +if (a2dp->lenbufe<codesize && a2dp->lenbufe+datatoread<sizeof (a2dp->bufe)){// if not enough data in bufe to encode and there is space in bufe +memcpy (a2dp->bufe+a2dp->lenbufe,buf,datatoread);// we read data to bufe + a2dp->lenbufe+=datatoread; }else {datatoread=0 ;}//nothing has been read -if (lenbufe>=codesize && a2dp->len + a2dp->sbc.len <678 ){// if enough data in bufe to encode and not enough frame to fill up mtu: encoding -change_endian (bufe,codesize);// changing the endianness - len =sbc_encode (&(a2dp->sbc), bufe, codesize);//encode -memmove (bufe, bufe + len, lenbufe - len);//shift the bufe - lenbufe-=len; - nbytes+=len; +if (a2dp->lenbufe>=codesize && a2dp->len + a2dp->sbc.len <678 ){// if enough data in bufe to encode and not enough frame to fill up mtu: encoding +change_endian (a2dp->bufe,codesize);// changing the endianness + len =sbc_encode (&(a2dp->sbc), a2dp->bufe, codesize);//encode +memmove (a2dp->bufe, a2dp->bufe + len, a2dp->lenbufe - len);//shift the bufe + a2dp->lenbufe-=len; + a2dp->nbytes+=len; sleeptime += a2dp->sbc.duration;if (len <=0 )return len; - frame_count++; + a2dp->frame_count++;memcpy (a2dp->buf + a2dp->len, a2dp->sbc.data, a2dp->sbc.len);// copy encoded frames into a2dp->buf a2dp->len+=a2dp->sbc.len;if (a2dp->state ==BT_CONNECTED )
@@ -692,25 +692,25 @@ }if (a2dp->len + a2dp->sbc.len >678 ){// if packet is formed - dt.tv_usec=1000000 *a2dp->sbc.subbands*a2dp->sbc.blocks*frame_count/io->rate;// time interval between transmitions + dt.tv_usec=1000000 *a2dp->sbc.subbands*a2dp->sbc.blocks*a2dp->frame_count/io->rate;// time interval between transmitions dt.tv_sec=0 ; -if (time_to_wait (&tsend, &dt)==0 ){// time to send data +if (time_to_wait (&a2dp->tsend, &dt)==0 ){// time to send data memset (&payload_header,0 ,sizeof (payload_header));// fill up the headers memset (&packet_header,0 ,sizeof (packet_header));//--- - payload_header.frame_count=frame_count; + payload_header.frame_count=a2dp->frame_count; packet_header.v =2 ; packet_header.pt =1 ; - packet_header.sequence_number =htons (seq_num); - packet_header.timestamp =htonl (timestamp); + packet_header.sequence_number =htons (a2dp->seq_num); + packet_header.timestamp =htonl (a2dp->timestamp); packet_header.ssrc =htonl (1 ); - timestamp += (a2dp->sbc.blocks +1 )*4 * (a2dp->sbc.subbands +1 )*4 ; + a2dp->timestamp += (a2dp->sbc.blocks +1 )*4 * (a2dp->sbc.subbands +1 )*4 ;memcpy (a2dp->buf, &packet_header,sizeof (packet_header));//copy the headers to buf memcpy (a2dp->buf +sizeof (packet_header), &payload_header,sizeof (payload_header));//--- write (a2dp->sk,a2dp->buf,a2dp->len);// sending the packet a2dp->len =sizeof (packet_header)+sizeof (payload_header);//inital position in buf, just after headers - frame_count=0 ; + a2dp->frame_count=0 ; sleeptime=0 ; - seq_num++; + a2dp->seq_num++; }else {usleep (1 );} }return datatoread / a2dp->frame_bytes;
@@ -731,51 +731,68 @@int written;struct timeval dt; - codesize=a2dp->sbc.subbands*a2dp->sbc.blocks*a2dp->sbc.channels*2 ; datatoread=min (codesize,size*a2dp->frame_bytes); + +struct timeval timeofday; +gettimeofday (&timeofday,NULL ); +/* +DBG("(%2d%3d): %d + %d * %d (=%d)", +(timeofday.tv_sec )%100, // s +(timeofday.tv_usec/1000)%1000, // ms +(int)areas->first, (int)areas->step, offset, (areas->first + areas->step * offset)); +/* +DBG("(%2d%3d): %d * %d * %d * 2 (=%d) x %d", +(timeofday.tv_sec )%100, // s +(timeofday.tv_usec/1000)%1000, // ms +a2dp->sbc.subbands, // 8 +a2dp->sbc.blocks, // 16 +a2dp->sbc.channels, // 2 +codesize, // 512 +datatoread); // 512 +*/ buf = (char *) areas->addr + (areas->first + areas->step * offset) /8 ; -if (lenbufe<codesize){ -memcpy (bufe+lenbufe,buf,datatoread); - lenbufe+=datatoread; +if (a2dp->lenbufe<codesize){ +memcpy (a2dp->bufe+a2dp->lenbufe,buf,datatoread); + a2dp->lenbufe+=datatoread; }else {datatoread=0 ;} -if (lenbufe>=codesize){//enough data to encode -change_endian (bufe,codesize);// changing the endianness - len =sbc_encode (&(a2dp->sbc), bufe, codesize);//encode -memmove (bufe, bufe + len, lenbufe - len);//shift the bufe - lenbufe-=len; - nbytes+=len; +if (a2dp->lenbufe>=codesize){//enough data to encode +change_endian (a2dp->bufe,codesize);// changing the endianness + len =sbc_encode (&(a2dp->sbc), a2dp->bufe, codesize);//encode +memmove (a2dp->bufe, a2dp->bufe + len, a2dp->lenbufe - len);//shift the bufe + a2dp->lenbufe-=len; + a2dp->nbytes+=len; sleeptime += a2dp->sbc.duration;if (len <=0 )return len;if (a2dp->len + a2dp->sbc.len >678 ) {// time to prepare and send the packet dt.tv_sec=0 ; - dt.tv_usec=1000000 *a2dp->sbc.subbands*a2dp->sbc.blocks*frame_count/io->rate; + dt.tv_usec=1000000 *a2dp->sbc.subbands*a2dp->sbc.blocks*a2dp->frame_count/io->rate;memset (&payload_header,0 ,sizeof (payload_header));memset (&packet_header,0 ,sizeof (packet_header)); - payload_header.frame_count=frame_count; + payload_header.frame_count=a2dp->frame_count; packet_header.v =2 ; packet_header.pt =1 ; - packet_header.sequence_number =htons (seq_num); - packet_header.timestamp =htonl (timestamp); + packet_header.sequence_number =htons (a2dp->seq_num); + packet_header.timestamp =htonl (a2dp->timestamp); packet_header.ssrc =htonl (1 ); - timestamp += (a2dp->sbc.blocks +1 )*4 * (a2dp->sbc.subbands +1 )*4 ; + a2dp->timestamp += (a2dp->sbc.blocks +1 )*4 * (a2dp->sbc.subbands +1 )*4 ;memcpy (a2dp->buf, &packet_header,sizeof (packet_header));memcpy (a2dp->buf +sizeof (packet_header), &payload_header,sizeof (payload_header)); -sleeptill (&tsend, &dt); +sleeptill (&a2dp->tsend, &dt);if ((written =write (a2dp->sk,a2dp->buf,a2dp->len)) != a2dp->len) {DBG ("Wrote %d not %d bytes; errno %s(%d)" , written, a2dp->len,strerror (errno), errno); } a2dp->len =sizeof (packet_header)+sizeof (payload_header); - frame_count=0 ; + a2dp->frame_count=0 ; sleeptime=0 ; - seq_num++; + a2dp->seq_num++; } - frame_count++; + a2dp->frame_count++;memcpy (a2dp->buf + a2dp->len, a2dp->sbc.data, a2dp->sbc.len); a2dp->len+=a2dp->sbc.len;if (a2dp->state ==BT_CONNECTED )
@@ -971,7 +988,7 @@unsigned int format[2 ], channel[2 ], rate[2 ];int err; -DBG ("a2dp %p" , a2dp); +DBG ("TEST a2dp %p" , a2dp); err =snd_pcm_ioplug_set_param_list (io,SND_PCM_IOPLUG_HW_ACCESS ,2 , access_list);if (err <0 )
@@ -1040,6 +1057,7 @@memset (a2dp,0 ,sizeof (*a2dp)); a2dp->refcnt =1 ; + a2dp->seq_num =1 ; a2dp->state =BT_OPEN ;
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Marcel Holtmann
maintainerHi Frederic,
Quoted 5 lines
Quoted 1 line
could you resend as a 'diff -u' output?
this is the command line I use is that correct?
cvs diff -u
pcm_a2dp.c> pcm_a2dp.c.diff-u
you should have a .cvsrc file in $HOME that looks like this:
cvs -z3
diff -u
update -dP
checkout -P
Quoted 1 line
Now the output has +- instead of ><
It is called unified diff and it is the preferred format around any open source project. I don't know of anybody who ever has reviewed a context diff.
Regards
Marcel
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Fr=E9d=E9ric
I had a look at your a2dp patches to move globals into the per-connection struct. It all looks like a clear improvement. I didn't have any problems with changes in quality after the change.
Would you like to check things out of cvs and verify it works for you?
thanks
Brad
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel
Quoted 9 lines
I had a look at your a2dp patches to move globals into the per-connection struct. It all looks like a clear improvement. I didn't have any problems with changes in quality after the change.
Would you like to check things out of cvs and verify it works for you?
=
Sorry for the long silence, I'm working on something else. I'm pleased =
you integrated my previous patch, it works well.
I tried two headset at a time. It does'nt crash, but I have bad sound. I =
believe this is due to bandwith limit. My bluetooth dongle is USB so =
there is enough bandwith. And bluetooth is 700 kb/s so I thought it =
should be enough to pass sbc data. I believed sbc was 128 kb/s. Maybe it =
is per channel... Can you confirm?
Thank you!
Fr=E9d=E9ric.
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel