NetBSD Problem Report #52196

From Manuel.Bouyer@lip6.fr  Wed Apr 26 16:52:25 2017
Return-Path: <Manuel.Bouyer@lip6.fr>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id E01727A2BB
	for <gnats-bugs@gnats.NetBSD.org>; Wed, 26 Apr 2017 16:52:25 +0000 (UTC)
Message-Id: <20170426165201.D7296A921@armandeche.soc.lip6.fr>
Date: Wed, 26 Apr 2017 18:52:01 +0200 (MEST)
From: bouyer@antioche.eu.org
Reply-To: bouyer@antioche.eu.org
To: gnats-bugs@NetBSD.org
Subject: ossaudio+portaudio2 fails on HEAD (with test prog)
X-Send-Pr-Version: 3.95

>Number:         52196
>Category:       kern
>Synopsis:       ossaudio+portaudio2 fails on HEAD
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    nat
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 26 16:55:00 +0000 2017
>Closed-Date:    Thu May 25 09:50:12 +0000 2017
>Last-Modified:  Thu May 25 09:50:12 +0000 2017
>Originator:     Manuel Bouyer
>Release:        HEAD as of today
>Organization:
-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--
>Environment:
System: NetBSD chartplotter 7.99.70 NetBSD 7.99.70 (CHARTPLOTTER) #7: Fri Apr 21 18:57:58 MEST 2017 evbarm
Architecture: earmv7hf
Machine: evbarm

>Description:
	OpenCPN (from pkgsrc) now wedge when playing sound.
	It worked file with older HEAD, and also works fine on netbsd-7.
	ktrace shows that it's writing the same 64k chunk of data over
	and over at a slow rate on the /dev/audio device.
	In case this matters this is on a allwinnner A20 evbarm board,
	with the awinac audio device. It supports only 48k, 16 or 24 bits.

	From the OopenCPN source I wrote a simple program which reproduces
	the problem, see below. You need audio/portaudio-devel from pkgsrc.
	Then compile with:
	gcc -o testpa -I/usr/pkg/include testpasound.c -Wl,-R/usr/pkg/lib/portaudio2 -L/usr/pkg/lib/portaudio2 -lportaudio

	and test with the 1bells.wav file from the opencpn source file
	(or download it from
	https://github.com/OpenCPN/OpenCPN/tree/master/data/sounds)
	./testpa /usr/pkg/share/opencpn/sounds/1bells.wav
	On netbsd-7 it plays fine, on HEAD it hangs (at last in my
	setup).


>How-To-Repeat:
	Here is the test program:

/******************************************************************************
 *
 * Project:  OpenCPN
 *
 ***************************************************************************
 *   Copyright (C) 2013 by David S. Register                               *
 *                                                                         *
 *   This program is free software; you can redistribute it and/or modify  *
 *   it under the terms of the GNU General Public License as published by  *
 *   the Free Software Foundation; either version 2 of the License, or     *
 *   (at your option) any later version.                                   *
 *                                                                         *
 *   This program is distributed in the hope that it will be useful,       *
 *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
 *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
 *   GNU General Public License for more details.                          *
 *                                                                         *
 *   You should have received a copy of the GNU General Public License     *
 *   along with this program; if not, write to the                         *
 *   Free Software Foundation, Inc.,                                       *
 *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,  USA.         *
 ***************************************************************************
 */

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>
#include <err.h>
#include <string.h>
#include <sys/stat.h>
#include <portaudio2/portaudio.h>

PaStream *stream;
void *sdata;
int sindex;
int smax_samples;
int splaying;

struct m_osdata {
	unsigned m_channels;
	unsigned m_samplingRate;
	unsigned m_bitsPerSample;
	unsigned m_samples;
	size_t m_dataBytes;
} m_osdata;

int LoadWAV(uint8_t *data, size_t length, int copyData);

/* This routine will be called by the PortAudio engine when audio is needed.
 * It may called at interrupt level on some machines so don't do anything
 * that could mess up the system like calling malloc() or free().
 */
static int OCPNSoundCallback( const void *inputBuffer, void *outputBuffer,
                           unsigned long framesPerBuffer,
                           const PaStreamCallbackTimeInfo* timeInfo,
                           PaStreamCallbackFlags statusFlags,
                           void *userData )
{
    /* Cast data passed through stream to our structure. */
    uint16_t *data = (uint16_t *)userData;
    uint16_t *out = (uint16_t*)outputBuffer;
    unsigned int i;
    int bdone = 0;
    (void) inputBuffer; /* Prevent unused variable warning. */
    for( i=0; i<framesPerBuffer; i++ )
    {
        *out = data[sindex];
        out++;
        sindex++;

        if( sindex >= smax_samples ) {
            bdone = 1;
            break;
        }
    }

    if(bdone)
        return paComplete;
    else
        return 0;
}

int
main(int argc, const char *argv[])
{
    int d;
    struct stat sb;
    PaError paerr = Pa_Initialize();
    if( paerr != paNoError )
        errx(1, "PortAudio CTOR error: %s\n", Pa_GetErrorText( paerr ) );

    if (argc != 2) {
	errx(1, "usage: testpa <filename>");
    }

    d = open(argv[1], O_RDONLY, 0);
    if (d < 0) {
	err(1, "open %s", argv[1]);
    }

    if (fstat(d, &sb) != 0) {
	err(1, "fstat");
    }

    size_t len = sb.st_size;
    char *data = malloc(len);
    if (data == NULL) {
	err(1, "malloc");
    }
    size_t rlen;
    size_t todo = len;
    char *buf = data;
    while (todo > 0) {
	    rlen = read(d, buf, todo);
	    if (rlen < 0) {
		err(1, "read");
	    }
	    if (rlen == 0) {
		errx(1, "short read");
	    }
	    buf += rlen;
	    todo -= rlen;
    }

    if ((rlen = LoadWAV(data, len, 0)) != 0)
    {
        errx(1, "Sound file is in unsupported format %d", rlen);
    }

    sindex = 0;
    smax_samples = m_osdata.m_samples;

    PaStreamParameters outputParameters;
    outputParameters.device = Pa_GetDefaultOutputDevice();
    outputParameters.channelCount = m_osdata.m_channels;
    outputParameters.sampleFormat = paInt16;
    outputParameters.suggestedLatency = 0;
    outputParameters.hostApiSpecificStreamInfo = NULL;

    PaStream *m_stream;
    /* Open an audio I/O stream. */
    paerr = Pa_OpenStream( &m_stream,
                         NULL, /* no input channels */
                         &outputParameters,
                         m_osdata.m_samplingRate,
                         256, /* frames per buffer, i.e. the number
                                 of sample frames that PortAudio will
                                 request from the callback. Many apps
                                 may want to use
                                 paFramesPerBufferUnspecified, which
                                 tells PortAudio to pick the best,
                                 possibly changing, buffer size.*/
                         paNoFlag, // flags
                         OCPNSoundCallback, /* this is your callback function */
                         sdata ); /*This is a pointer that will be passed to
                                    your callback*/
    if( paerr != paNoError )
        errx(1, "PortAudio Create() error: %s", Pa_GetErrorText( paerr ) );

    Pa_StopStream( m_stream );

    sindex = 0;
    smax_samples = m_osdata.m_samples;

    paerr = Pa_StartStream( m_stream );
    if( paerr != paNoError ) {
         errx(1,  "PortAudio Play() error: %s\n", Pa_GetErrorText( paerr ) );
    }
    sleep(10);
    exit(0);
}

typedef struct
{
    uint32_t      uiSize;
    uint16_t      uiFormatTag;
    uint16_t      uiChannels;
    uint32_t      ulSamplesPerSec;
    uint32_t      ulAvgBytesPerSec;
    uint16_t      uiBlockAlign;
    uint16_t      uiBitsPerSample;
} WAVEFORMAT;

#define WAVE_FORMAT_PCM  1
#define WAVE_INDEX       8
#define FMT_INDEX       12

int
LoadWAV(uint8_t *data, size_t length, int copyData)
{
    // the simplest wave file header consists of 44 bytes:
    //
    //      0   "RIFF"
    //      4   file size - 8
    //      8   "WAVE"
    //
    //      12  "fmt "
    //      16  chunk size                  |
    //      20  format tag                  |
    //      22  number of channels          |
    //      24  sample rate                 | WAVEFORMAT
    //      28  average bytes per second    |
    //      32  bytes per frame             |
    //      34  bits per sample             |
    //
    //      36  "data"
    //      40  number of data bytes
    //      44  (wave signal) data
    //
    // so check that we have at least as much
    if ( length < 44 )
        return 1;

    WAVEFORMAT waveformat;
    memcpy(&waveformat, &data[FMT_INDEX + 4], sizeof(WAVEFORMAT));

    //  Sanity checks
    if (memcmp(data, "RIFF", 4) != 0)
        return 2;
    if (memcmp(&data[WAVE_INDEX], "WAVE", 4) != 0)
        return 3;
    if (memcmp(&data[FMT_INDEX], "fmt ", 4) != 0)
        return 4;

    // get the sound data size
        size_t ul = 0;
    //  Get the "data" chunk length
    if (memcmp(&data[FMT_INDEX + waveformat.uiSize + 8], "data", 4) == 0) {
        memcpy(&ul, &data[FMT_INDEX + waveformat.uiSize + 12], 4);
    }

    //  There may be a "fact" chunk in the header, which will displace the first "data" chunk
    //  If so, find the "data" chunk 12 bytes further along
    else if (memcmp(&data[FMT_INDEX + waveformat.uiSize + 8], "fact", 4) == 0) {
        if (memcmp(&data[FMT_INDEX + waveformat.uiSize + 8 + 12], "data", 4) == 0) {
            memcpy(&ul, &data[FMT_INDEX + waveformat.uiSize + 12 + 12], 4);
        }
    }

    if ( length < ul + FMT_INDEX + waveformat.uiSize + 16 ) {
	printf("length %d < %d\n", length, ul + FMT_INDEX + waveformat.uiSize + 16);
        return 5;
    }

    if (waveformat.uiFormatTag != WAVE_FORMAT_PCM)
        return 6;

    if (waveformat.ulSamplesPerSec !=
        waveformat.ulAvgBytesPerSec / waveformat.uiBlockAlign)
        return 7;

    m_osdata.m_channels = waveformat.uiChannels;
    m_osdata.m_samplingRate = waveformat.ulSamplesPerSec;
    m_osdata.m_bitsPerSample = waveformat.uiBitsPerSample;
    m_osdata.m_samples = ul / (m_osdata.m_channels * m_osdata.m_bitsPerSample / 8);
    m_osdata.m_dataBytes = ul;

    sdata = (&data[FMT_INDEX + waveformat.uiSize + 8]);

    return 0;
}

>Fix:
	unknown

>Release-Note:

>Audit-Trail:
From: "Nathanial Sloss" <nat@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52196 CVS commit: pkgsrc/audio/portaudio-devel
Date: Thu, 27 Apr 2017 07:14:01 +0000

 Module Name:	pkgsrc
 Committed By:	nat
 Date:		Thu Apr 27 07:14:01 UTC 2017

 Modified Files:
 	pkgsrc/audio/portaudio-devel: Makefile distinfo
 Added Files:
 	pkgsrc/audio/portaudio-devel/patches:
 	    patch-src_hostapi_oss_pa__unix__oss.c

 Log Message:
 Don't write an endless stream of silence whilst preparing playback.
 Bump PKGREVISION.

 Addresses PR kern/52196.


 To generate a diff of this commit:
 cvs rdiff -u -r1.12 -r1.13 pkgsrc/audio/portaudio-devel/Makefile
 cvs rdiff -u -r1.9 -r1.10 pkgsrc/audio/portaudio-devel/distinfo
 cvs rdiff -u -r0 -r1.1 \
     pkgsrc/audio/portaudio-devel/patches/patch-src_hostapi_oss_pa__unix__oss.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: PR/52196 CVS commit: pkgsrc/audio/portaudio-devel
Date: Thu, 27 Apr 2017 11:23:26 +0200

 On Thu, Apr 27, 2017 at 07:15:01AM +0000, Nathanial Sloss wrote:
 > The following reply was made to PR kern/52196; it has been noted by GNATS.
 > 
 > From: "Nathanial Sloss" <nat@netbsd.org>
 > To: gnats-bugs@gnats.NetBSD.org
 > Cc: 
 > Subject: PR/52196 CVS commit: pkgsrc/audio/portaudio-devel
 > Date: Thu, 27 Apr 2017 07:14:01 +0000
 > 
 >  Module Name:	pkgsrc
 >  Committed By:	nat
 >  Date:		Thu Apr 27 07:14:01 UTC 2017
 >  
 >  Modified Files:
 >  	pkgsrc/audio/portaudio-devel: Makefile distinfo
 >  Added Files:
 >  	pkgsrc/audio/portaudio-devel/patches:
 >  	    patch-src_hostapi_oss_pa__unix__oss.c
 >  
 >  Log Message:
 >  Don't write an endless stream of silence whilst preparing playback.
 >  Bump PKGREVISION.
 >  
 >  Addresses PR kern/52196.

 This indeed fixes the problem with my test program, and with opencpn.
 thanks a lot !

 Any idea why it stopped working in HEAD ? It looks like the write() should
 return -1 with EWOULDBLOCK at some point, but for some reason it doesn't
 happens. Maybe nonblocking writes to /dev/audio is not working anymore as
 it should ?

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: Nathanial Sloss <nat@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/52196
Date: Thu, 27 Apr 2017 20:19:39 +1000

 I think it would return EWOULDBLOCK if the write was larger.

 What I noticed with a kernel with the old audio code was that there was a 
 delay of a second or so before sound would start to play.

 I'm guessing that even with old audio it would take some time before returning 
 EWOULDBLOCK.

 Best regards,

 Nat

From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@NetBSD.org
Cc: kern-bug-people@netbsd.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: kern/52196
Date: Thu, 27 Apr 2017 12:30:37 +0200

 On Thu, Apr 27, 2017 at 10:25:01AM +0000, Nathanial Sloss wrote:
 > The following reply was made to PR kern/52196; it has been noted by GNATS.
 > 
 > From: Nathanial Sloss <nat@netbsd.org>
 > To: gnats-bugs@netbsd.org
 > Cc: 
 > Subject: Re: kern/52196
 > Date: Thu, 27 Apr 2017 20:19:39 +1000
 > 
 >  I think it would return EWOULDBLOCK if the write was larger.

 But is that expected ? I would expect the write syscall to return
 immediately in all case when the fd is set to non-blocking I/O
 >  
 >  What I noticed with a kernel with the old audio code was that there was a 
 >  delay of a second or so before sound would start to play.
 >  
 >  I'm guessing that even with old audio it would take some time before returning 
 >  EWOULDBLOCK.

 or maybe it's just that the buffer has been filled with 1s of silence,
 and it has to drain ?

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

From: "Benny Siegert" <bsiegert@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52196 CVS commit: [pkgsrc-2017Q1] pkgsrc/audio/portaudio-devel
Date: Mon, 1 May 2017 09:25:40 +0000

 Module Name:	pkgsrc
 Committed By:	bsiegert
 Date:		Mon May  1 09:25:40 UTC 2017

 Modified Files:
 	pkgsrc/audio/portaudio-devel [pkgsrc-2017Q1]: Makefile distinfo
 Added Files:
 	pkgsrc/audio/portaudio-devel/patches [pkgsrc-2017Q1]:
 	    patch-src_hostapi_oss_pa__unix__oss.c

 Log Message:
 Pullup ticket #5378 - requested by sevan
 audio/portaudio-devel: bugfix

 Revisions pulled up:
 - audio/portaudio-devel/Makefile                                1.13
 - audio/portaudio-devel/distinfo                                1.10
 - audio/portaudio-devel/patches/patch-src_hostapi_oss_pa__unix__oss.c 1.1

 ---
    Module Name:    pkgsrc
    Committed By:   nat
    Date:           Thu Apr 27 07:14:01 UTC 2017

    Modified Files:
            pkgsrc/audio/portaudio-devel: Makefile distinfo
    Added Files:
            pkgsrc/audio/portaudio-devel/patches:
                patch-src_hostapi_oss_pa__unix__oss.c

    Log Message:
    Don't write an endless stream of silence whilst preparing playback.
    Bump PKGREVISION.

    Addresses PR kern/52196.


 To generate a diff of this commit:
 cvs rdiff -u -r1.12 -r1.12.2.1 pkgsrc/audio/portaudio-devel/Makefile
 cvs rdiff -u -r1.9 -r1.9.2.1 pkgsrc/audio/portaudio-devel/distinfo
 cvs rdiff -u -r0 -r1.1.2.2 \
     pkgsrc/audio/portaudio-devel/patches/patch-src_hostapi_oss_pa__unix__oss.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Nathanial Sloss" <nat@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/52196 CVS commit: src/sys/dev
Date: Tue, 2 May 2017 06:25:05 +0000

 Module Name:	src
 Committed By:	nat
 Date:		Tue May  2 06:25:05 UTC 2017

 Modified Files:
 	src/sys/dev: audio.c

 Log Message:
 Explicitly set ioflag in audio_[read/write] IO_NDELAY if O_NONBLOCK is set
 in f_flags.  This makes nonblocking reads and writes behave normally.

 Addresses PR kern/52196.


 To generate a diff of this commit:
 cvs rdiff -u -r1.329 -r1.330 src/sys/dev/audio.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: kern-bug-people->nat
Responsible-Changed-By: nat@NetBSD.org
Responsible-Changed-When: Wed, 03 May 2017 03:38:22 +0000
Responsible-Changed-Why:
Take.


State-Changed-From-To: open->feedback
State-Changed-By: nat@NetBSD.org
State-Changed-When: Wed, 03 May 2017 03:38:22 +0000
State-Changed-Why:
This has been fixed in many ways both audio.c and package.
OK to close?


From: Manuel Bouyer <bouyer@antioche.eu.org>
To: gnats-bugs@NetBSD.org
Cc: nat@NetBSD.org, kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org,
        gnats-admin@netbsd.org
Subject: Re: kern/52196 (ossaudio+portaudio2 fails on HEAD)
Date: Wed, 3 May 2017 09:31:55 +0200

 On Wed, May 03, 2017 at 03:38:22AM +0000, nat@NetBSD.org wrote:
 > State-Changed-From-To: open->feedback
 > State-Changed-By: nat@NetBSD.org
 > State-Changed-When: Wed, 03 May 2017 03:38:22 +0000
 > State-Changed-Why:
 > This has been fixed in many ways both audio.c and package.
 > OK to close?

 The package change works fine. I can't test the audio.c change before
 the end of next week.

 -- 
 Manuel Bouyer <bouyer@antioche.eu.org>
      NetBSD: 26 ans d'experience feront toujours la difference
 --

State-Changed-From-To: feedback->closed
State-Changed-By: nat@NetBSD.org
State-Changed-When: Thu, 25 May 2017 09:50:12 +0000
State-Changed-Why:
Confirmed fixed.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.