From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 1/2] ALSA: usb: fix corrupted pointers due to interface setting change Date: Fri, 14 Aug 2015 17:28:27 +0200 Message-ID: References: <1439505753-3532-1-git-send-email-pierre-louis.bossart@linux.intel.com> <1439505753-3532-2-git-send-email-pierre-louis.bossart@linux.intel.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id 23E0E2654B8 for ; Fri, 14 Aug 2015 17:28:29 +0200 (CEST) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Pierre-Louis Bossart Cc: alsa-devel@alsa-project.org List-Id: alsa-devel@alsa-project.org On Fri, 14 Aug 2015 17:03:10 +0200, Takashi Iwai wrote: > > On Fri, 14 Aug 2015 00:42:32 +0200, > Pierre-Louis Bossart wrote: > > > > When a transition occurs between alternate settings that do not use the > > same synchronization method, the substream pointers were not reset. > > This prevents audio from being played during the second transition. > > > > Identified and tested with M-Audio Transit device > > (0763:2006 Midiman M-Audio Transit) > > Hmm, I have this old device, too, but couldn't reproduce the problem. > Is there any special setup with it? OK, I could see it after switching a few times. But, also your patch didn't cure, either... Takashi > > > Takashi > > > > > Details of the issue: > > > > First playback to adaptive endpoint: > > $ aplay -Dhw:1,0 ~/24_96.wav > > Playing WAVE '/home/plb/24_96.wav' : Signed 24 bit Little Endian in 3bytes, > > Rate 96000 Hz, Stereo > > > > [ 3169.297556] usb 1-2: setting usb interface 1:1 > > [ 3169.297568] usb 1-2: Creating new playback data endpoint #3 > > [ 3169.298563] usb 1-2: Setting params for ep #3 (type 0, 3 urbs), ret=0 > > [ 3169.298574] usb 1-2: Starting data EP @ffff880035fc8000 > > > > first playback to asynchronous endpoint: > > $ aplay -Dhw:1,0 ~/16_48.wav > > Playing WAVE '/home/plb/16_48.wav' : Signed 16 bit Little Endian, > > Rate 48000 Hz, Stereo > > > > [ 3204.520251] usb 1-2: setting usb interface 1:3 > > [ 3204.520264] usb 1-2: Creating new playback data endpoint #3 > > [ 3204.520272] usb 1-2: Creating new capture sync endpoint #83 > > [ 3204.521162] usb 1-2: Setting params for ep #3 (type 0, 4 urbs), ret=0 > > [ 3204.521177] usb 1-2: Setting params for ep #83 (type 1, 4 urbs), ret=0 > > [ 3204.521182] usb 1-2: Starting data EP @ffff880035fce000 > > [ 3204.521204] usb 1-2: Starting sync EP @ffff8800bd616000 > > > > second playback to adaptive endpoint: no audio and error on terminal: > > $ aplay -Dhw:1,0 ~/24_96.wav > > Playing WAVE '/home/plb/24_96.wav' : Signed 24 bit Little Endian in 3bytes, > > Rate 96000 Hz, Stereo > > aplay: pcm_write:1939: write error: Input/output error > > > > [ 3239.483589] usb 1-2: setting usb interface 1:1 > > [ 3239.483601] usb 1-2: Re-using EP 3 in iface 1,1 @ffff880035fc8000 > > [ 3239.484590] usb 1-2: Setting params for ep #3 (type 0, 4 urbs), ret=0 > > [ 3239.484606] usb 1-2: Setting params for ep #83 (type 1, 4 urbs), ret=0 > > > > This last line shows that a sync endpoint is used when it shouldn't. > > The sync endpoint is no longer valid and the pointers are corrupted > > > > Signed-off-by: Pierre-Louis Bossart > > --- > > sound/usb/pcm.c | 13 +++++++++++++ > > 1 file changed, 13 insertions(+) > > > > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c > > index b4ef410..7cd7c03 100644 > > --- a/sound/usb/pcm.c > > +++ b/sound/usb/pcm.c > > @@ -395,6 +395,19 @@ static int set_sync_endpoint(struct snd_usb_substream *subs, > > if (err < 0) > > return err; > > > > + if ((is_playback && (attr != USB_ENDPOINT_SYNC_ASYNC)) || > > + (!is_playback && (attr != USB_ENDPOINT_SYNC_ADAPTIVE))) { > > + > > + /* > > + * Clean-up subs pointers to make sure sync_endpoint is never > > + * configured. This is needed in case of a transition between > > + * alternate settings using different synchronization modes > > + * where the previous sync_endpoint may no longer be valid. > > + */ > > + subs->sync_endpoint = NULL; > > + subs->data_endpoint->sync_master = NULL; > > + } > > + > > if (altsd->bNumEndpoints < 2) > > return 0; > > > > -- > > 1.9.1 > >