NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/54474: Jetson TK1 audio playback has clicks since isaki-audio2 merge



The following reply was made to PR kern/54474; it has been noted by GNATS.

From: Nick Hudson <nick.hudson%gmx.co.uk@localhost>
To: Tetsuya Isaki <isaki%pastel-flower.jp@localhost>,
 Nick Hudson <nick.hudson%gmx.co.uk@localhost>
Cc: gnats-bugs%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Subject: Re: kern/54474: Jetson TK1 audio playback has clicks since
 isaki-audio2 merge
Date: Sat, 31 Aug 2019 21:51:23 +0100

 This is a multi-part message in MIME format.
 --------------073DC74152DF68FBA935189B
 Content-Type: text/plain; charset=utf-8; format=flowed
 Content-Transfer-Encoding: quoted-printable
 
 On 24/08/2019 05:47, Tetsuya Isaki wrote:
 > At Sun, 18 Aug 2019 16:53:08 +0100,
 > Nick Hudson wrote:
 >> That said, the click still exists on netbsd-9 - here's the output from =
 a
 >> netbsd-9 system
 >>
 >> # audiocfg test 0
 >> 0: [*] audio0 @ hdafg0: NVIDIA Tegra124 HDMI
 >>          playback: 8ch, 44100Hz
 >>          record:   2ch, 44100Hz
 >>          (P-) slinear_le 16/16, 2ch, { 44100 }
 >>          (P-) slinear_le 16/16, 4ch, { 44100 }
 >>          (P-) slinear_le 16/16, 6ch, { 44100 }
 >>          (P-) slinear_le 16/16, 8ch, { 44100 }
 >>          (PR) slinear_le 16/16, 2ch, 44100-44100Hz
 >>     testing channel 0... done
 >>     testing channel 1... done
 >>     testing channel 2... done
 >>     testing channel 3... done
 >>     testing channel 4... done
 >>     testing channel 5... done
 >>     testing channel 6... done
 >>     testing channel 7...write: Resource temporarily unavailable
 >> #
 >
 > Please try this commit.
 > If you can play audio other than "audiocfg test", I hope it fixes
 > reported problem.
 >
 > | Committed By:	isaki
 > | Date:		Sat Aug 24 04:04:10 UTC 2019
 > |
 > | Modified Files:
 > | 	src/usr.bin/audiocfg: audiodev.c audiodev.h main.c
 > |
 > | Log Message:
 > | Revert to use single descriptor for "audiocfg test" as before.
 > |
 > |
 > | To generate a diff of this commit:
 > | cvs rdiff -u -r1.8 -r1.9 src/usr.bin/audiocfg/audiodev.c
 > | cvs rdiff -u -r1.6 -r1.7 src/usr.bin/audiocfg/audiodev.h
 > | cvs rdiff -u -r1.10 -r1.11 src/usr.bin/audiocfg/main.c
 
 Here's the output with the above changes and the previous "NFCI" change
 applied to netbsd-9 (see attached diff)
 
 
 tk1# audiocfg test 0
 0: [*] audio0 @ hdafg0: NVIDIA Tegra124 HDMI
         playback: 8ch, 44100Hz
         record:   2ch, 44100Hz
         (P-) slinear_le 16/16, 2ch, { 44100 }
         (P-) slinear_le 16/16, 4ch, { 44100 }
         (P-) slinear_le 16/16, 6ch, { 44100 }
         (P-) slinear_le 16/16, 8ch, { 44100 }
         (PR) slinear_le 16/16, 2ch, 44100-44100Hz
    testing channel 0... done
    testing channel 1... done
    testing channel 2... done
    testing channel 3... done
    testing channel 4... done
    testing channel 5... done
    testing channel 6... done
    testing channel 7... done
 tk1#
 
 
 The audio is still "clicky"/distorted in some way.
 
 
 > And please test this sample code as well.  This code just plays four
 > seconds silence to /dev/audio0.  If isaki-audio2 driver is wrong,
 > it will only fail on >=3D8.99.39 kernel and will not fail on <8.99.39.
 > If it fails on both, it may be hardware driver(hdafg)'s issue.
 >
 
 [rept.c deleted]
 
 I couldn't hear any audio with this, but then the tk1's fan is really nois=
 y.
 
 Something is still no quite right with audiocfg test audio output.
 
 Nick
 
 --------------073DC74152DF68FBA935189B
 Content-Type: text/x-patch;
  name="audiocfg.diff"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment;
  filename="audiocfg.diff"
 
 Index: usr.bin/audiocfg/audiodev.c
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 RCS file: /cvsroot/src/usr.bin/audiocfg/audiodev.c,v
 retrieving revision 1.7
 diff -u -p -r1.7 audiodev.c
 =2D-- usr.bin/audiocfg/audiodev.c	8 May 2019 14:36:12 -0000	1.7
 +++ usr.bin/audiocfg/audiodev.c	31 Aug 2019 20:47:07 -0000
 @@ -43,6 +43,9 @@
  #include "drvctl.h"
  #include "dtmf.h"
 
 +static int audiodev_test_chmask(struct audiodev *, unsigned int,
 +	audio_info_t *);
 +
  static TAILQ_HEAD(audiodevhead, audiodev) audiodevlist =3D
      TAILQ_HEAD_INITIALIZER(audiodevlist);
 
 @@ -63,26 +66,26 @@ audiodev_getinfo(struct audiodev *adev)
  	if (stat(_PATH_AUDIOCTL, &st) !=3D -1 && st.st_rdev =3D=3D adev->dev)
  		adev->defaultdev =3D true;
 
 -	adev->fd =3D open(adev->ctlpath, O_RDONLY);
 -	if (adev->fd =3D=3D -1) {
 +	adev->ctlfd =3D open(adev->ctlpath, O_RDONLY);
 +	if (adev->ctlfd =3D=3D -1) {
  			return -1;
  	}
 -	if (ioctl(adev->fd, AUDIO_GETDEV, &adev->audio_device) =3D=3D -1) {
 -		close(adev->fd);
 +	if (ioctl(adev->ctlfd, AUDIO_GETDEV, &adev->audio_device) =3D=3D -1) {
 +		close(adev->ctlfd);
  		return -1;
  	}
 
  	for (i =3D 0; ;i++) {
  		memset(&query, 0, sizeof(query));
  		query.index =3D i;
 -		if (ioctl(adev->fd, AUDIO_QUERYFORMAT, &query) =3D=3D -1) {
 +		if (ioctl(adev->ctlfd, AUDIO_QUERYFORMAT, &query) =3D=3D -1) {
  			if (errno =3D=3D ENODEV) {
  				/* QUERYFORMAT not supported. */
  				break;
  			}
  			if (errno =3D=3D EINVAL)
  				break;
 -			close(adev->fd);
 +			close(adev->ctlfd);
  			return -1;
  		}
 
 @@ -91,8 +94,8 @@ audiodev_getinfo(struct audiodev *adev)
  		TAILQ_INSERT_TAIL(&adev->formats, f, next);
  	}
 
 -	if (ioctl(adev->fd, AUDIO_GETFORMAT, &adev->info) =3D=3D -1) {
 -		close(adev->fd);
 +	if (ioctl(adev->ctlfd, AUDIO_GETFORMAT, &adev->hwinfo) =3D=3D -1) {
 +		close(adev->ctlfd);
  		return -1;
  	}
 
 @@ -158,8 +161,8 @@ audiodev_refresh(void)
 
  	while (!TAILQ_EMPTY(&audiodevlist)) {
  		adev =3D TAILQ_FIRST(&audiodevlist);
 -		if (adev->fd !=3D -1)
 -			close(adev->fd);
 +		if (adev->ctlfd !=3D -1)
 +			close(adev->ctlfd);
  		TAILQ_REMOVE(&audiodevlist, adev, next);
  		free(adev);
  	}
 @@ -250,12 +253,12 @@ int
  audiodev_set_param(struct audiodev *adev, int mode,
  	const char *encname, unsigned int prec, unsigned int ch, unsigned int fr=
 eq)
  {
 -	struct audio_info ai;
 +	audio_info_t ai;
  	int setmode;
  	u_int enc;
 
  	setmode =3D 0;
 -	ai =3D adev->info;
 +	ai =3D adev->hwinfo;
 
  	for (enc =3D 0; enc < encoding_max; enc++) {
  		if (strcmp(encname, encoding_names[enc]) =3D=3D 0)
 @@ -290,7 +293,7 @@ audiodev_set_param(struct audiodev *adev
  	ai.mode =3D setmode;
  	printf("setting %s to %s:%u, %uch, %uHz\n",
  	    adev->xname, encname, prec, ch, freq);
 -	if (ioctl(adev->fd, AUDIO_SETFORMAT, &ai) =3D=3D -1) {
 +	if (ioctl(adev->ctlfd, AUDIO_SETFORMAT, &ai) =3D=3D -1) {
  		perror("ioctl AUDIO_SETFORMAT");
  		return -1;
  	}
 @@ -298,40 +301,64 @@ audiodev_set_param(struct audiodev *adev
  }
 
  int
 -audiodev_test(struct audiodev *adev, unsigned int chanmask)
 +audiodev_test(struct audiodev *adev)
  {
  	audio_info_t info;
 -	int16_t *buf;
 -	size_t buflen;
 -	off_t off;
 -	int fd;
 -	int rv =3D -1;
 +	unsigned int i;
 +	int rv;
 
 -	fd =3D open(adev->path, O_WRONLY);
 -	if (fd =3D=3D -1) {
 +	rv =3D -1;
 +
 +	adev->fd =3D open(adev->path, O_WRONLY);
 +	if (adev->fd =3D=3D -1) {
  		perror("open");
  		return -1;
  	}
 
  	AUDIO_INITINFO(&info);
  	info.play.sample_rate =3D AUDIODEV_SAMPLE_RATE;
 -	info.play.channels =3D adev->info.play.channels;
 +	info.play.channels =3D adev->hwinfo.play.channels;
  	info.play.precision =3D 16;
  	info.play.encoding =3D AUDIO_ENCODING_SLINEAR_LE;
  	info.mode =3D AUMODE_PLAY;
 -	if (ioctl(fd, AUDIO_SETINFO, &info) =3D=3D -1) {
 +	if (ioctl(adev->fd, AUDIO_SETINFO, &info) =3D=3D -1) {
  		perror("ioctl AUDIO_SETINFO");
 -		goto abort;
 +		goto done;
  	}
 -	if (ioctl(fd, AUDIO_GETINFO, &info) =3D=3D -1) {
 +	if (ioctl(adev->fd, AUDIO_GETINFO, &info) =3D=3D -1) {
  		perror("ioctl AUDIO_GETINFO");
 -		goto abort;
 +		goto done;
  	}
 
 -	dtmf_new(&buf, &buflen, info.play.sample_rate, 2,
 -	    adev->info.play.channels, chanmask, 350.0, 440.0);
 +	for (i =3D 0; i < adev->hwinfo.play.channels; i++) {
 +		printf("  testing channel %u...", i);
 +		fflush(stdout);
 +		if (audiodev_test_chmask(adev, 1 << i, &info) =3D=3D -1)
 +			goto done;
 +		printf(" done\n");
 +	}
 +
 +	rv =3D 0;
 +done:
 +	close(adev->fd);
 +	return rv;
 +}
 +
 +static int
 +audiodev_test_chmask(struct audiodev *adev, unsigned int chanmask,
 +	audio_info_t *info)
 +{
 +	int16_t *buf;
 +	size_t buflen;
 +	off_t off;
 +	int rv;
 +
 +	rv =3D -1;
 +
 +	dtmf_new(&buf, &buflen, info->play.sample_rate, 2,
 +	    adev->hwinfo.play.channels, chanmask, 350.0, 440.0);
  	if (buf =3D=3D NULL) {
 -		goto abort;
 +		return -1;
  	}
 
  	off =3D 0;
 @@ -339,10 +366,10 @@ audiodev_test(struct audiodev *adev, uns
  		size_t wlen;
  		ssize_t ret;
 
 -		wlen =3D info.play.buffer_size;
 +		wlen =3D info->play.buffer_size;
  		if (wlen > buflen)
  			wlen =3D buflen;
 -		ret =3D write(fd, (char *)buf + off, wlen);
 +		ret =3D write(adev->fd, (char *)buf + off, wlen);
  		if (ret =3D=3D -1) {
  			perror("write");
  			goto done;
 @@ -352,7 +379,7 @@ audiodev_test(struct audiodev *adev, uns
  		buflen -=3D wlen;
  	}
 
 -	if (ioctl(fd, AUDIO_DRAIN) =3D=3D -1) {
 +	if (ioctl(adev->fd, AUDIO_DRAIN) =3D=3D -1) {
  		perror("ioctl AUDIO_DRAIN");
  		goto done;
  	}
 @@ -360,8 +387,5 @@ audiodev_test(struct audiodev *adev, uns
  	rv =3D 0;
  done:
  	free(buf);
 -abort:
 -	close(fd);
 -
  	return rv;
  }
 Index: usr.bin/audiocfg/audiodev.h
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 RCS file: /cvsroot/src/usr.bin/audiocfg/audiodev.h,v
 retrieving revision 1.5
 diff -u -p -r1.5 audiodev.h
 =2D-- usr.bin/audiocfg/audiodev.h	8 May 2019 14:36:12 -0000	1.5
 +++ usr.bin/audiocfg/audiodev.h	31 Aug 2019 20:47:07 -0000
 @@ -46,14 +46,15 @@ struct audiodev {
  	uint16_t unit;
  	char path[PATH_MAX+1];
  	char ctlpath[PATH_MAX+1];
 -
  	int fd;
 +	int ctlfd;
 +
  	dev_t dev;
  	bool defaultdev;
 
  	audio_device_t audio_device;
  	TAILQ_HEAD(, audiofmt) formats;
 -	struct audio_info info;
 +	audio_info_t hwinfo;
 
  	TAILQ_ENTRY(audiodev) next;
  };
 @@ -65,7 +66,7 @@ int			audiodev_set_default(struct audiod
  int			audiodev_set_param(struct audiodev *, int,
  				const char *, unsigned int, unsigned int,
  				unsigned int);
 -int			audiodev_test(struct audiodev *, unsigned int);
 +int			audiodev_test(struct audiodev *);
 
  extern const char *	encoding_names[];
  extern u_int		encoding_max;
 Index: usr.bin/audiocfg/main.c
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 RCS file: /cvsroot/src/usr.bin/audiocfg/main.c,v
 retrieving revision 1.8
 diff -u -p -r1.8 main.c
 =2D-- usr.bin/audiocfg/main.c	8 May 2019 14:36:12 -0000	1.8
 +++ usr.bin/audiocfg/main.c	31 Aug 2019 20:47:07 -0000
 @@ -87,17 +87,21 @@ print_audiodev(struct audiodev *adev, in
  		printf(" %s", adev->audio_device.version);
  	printf("\n");
  	printf("       playback: ");
 -	if ((adev->info.mode & AUMODE_PLAY))
 +	if ((adev->hwinfo.mode & AUMODE_PLAY)) {
  		printf("%uch, %uHz\n",
 -		    adev->info.play.channels, adev->info.play.sample_rate);
 -	else
 +		    adev->hwinfo.play.channels,
 +		    adev->hwinfo.play.sample_rate);
 +	} else {
  		printf("unavailable\n");
 +	}
  	printf("       record:   ");
 -	if ((adev->info.mode & AUMODE_RECORD))
 +	if ((adev->hwinfo.mode & AUMODE_RECORD)) {
  		printf("%uch, %uHz\n",
 -		    adev->info.record.channels, adev->info.record.sample_rate);
 -	else
 +		    adev->hwinfo.record.channels,
 +		    adev->hwinfo.record.sample_rate);
 +	} else {
  		printf("unavailable\n");
 +	}
 
  	TAILQ_FOREACH(f, &adev->formats, next) {
  		printf("       ");
 @@ -244,13 +248,8 @@ main(int argc, char *argv[])
  			return EXIT_FAILURE;
  		}
  		print_audiodev(adev, i);
 -		for (i =3D 0; i < adev->info.play.channels; i++) {
 -			printf("  testing channel %d...", i);
 -			fflush(stdout);
 -			if (audiodev_test(adev, 1 << i) =3D=3D -1)
 -				return EXIT_FAILURE;
 -			printf(" done\n");
 -		}
 +		if (audiodev_test(adev) =3D=3D -1)
 +			return EXIT_FAILURE;
  	} else
  		usage(argv[0]);
  		/* NOTREACHED */
 
 --------------073DC74152DF68FBA935189B--
 


Home | Main Index | Thread Index | Old Index