* [PATCH] pcm: route: Don't handle no matching chmap as a serious error @ 2014-03-18 14:27 Takashi Iwai 2014-03-18 16:19 ` David Henningsson 0 siblings, 1 reply; 7+ messages in thread From: Takashi Iwai @ 2014-03-18 14:27 UTC (permalink / raw) To: alsa-devel; +Cc: David Henningsson When find_matching_chmap() returns an error for the non-matching chmap, the caller, snd_pcm_route_open(), also returns an error although it shouldn't be handled as the fatal error. This results in the probe error with PulseAudio and it gives no real output in the end. Signed-off-by: Takashi Iwai <tiwai@suse.de> --- src/pcm/pcm_route.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c index ab17fa78be2c..ac11bdc8adfd 100644 --- a/src/pcm/pcm_route.c +++ b/src/pcm/pcm_route.c @@ -940,10 +940,8 @@ static int find_matching_chmap(snd_pcm_t *spcm, snd_pcm_chmap_t *tt_chmap, snd_pcm_free_chmaps(chmaps); - if (*found_chmap == NULL) { + if (*found_chmap == NULL) SNDERR("Found no matching channel map"); - return -EINVAL; - } return 0; } -- 1.9.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] pcm: route: Don't handle no matching chmap as a serious error 2014-03-18 14:27 [PATCH] pcm: route: Don't handle no matching chmap as a serious error Takashi Iwai @ 2014-03-18 16:19 ` David Henningsson 2014-03-18 16:34 ` Takashi Iwai 0 siblings, 1 reply; 7+ messages in thread From: David Henningsson @ 2014-03-18 16:19 UTC (permalink / raw) To: Takashi Iwai, alsa-devel On 03/18/2014 03:27 PM, Takashi Iwai wrote: > When find_matching_chmap() returns an error for the non-matching > chmap, the caller, snd_pcm_route_open(), also returns an error > although it shouldn't be handled as the fatal error. This results in > the probe error with PulseAudio and it gives no real output in the > end. Hmm, could you give a more specific example? In case the driver does not support channel maps at all, that case is handled in the beginning of the function. So this only happens if the driver supports channel maps, but only non-compatible with the requested map. In which case I believe it's correct that the probing should fail...? > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > src/pcm/pcm_route.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c > index ab17fa78be2c..ac11bdc8adfd 100644 > --- a/src/pcm/pcm_route.c > +++ b/src/pcm/pcm_route.c > @@ -940,10 +940,8 @@ static int find_matching_chmap(snd_pcm_t *spcm, snd_pcm_chmap_t *tt_chmap, > > snd_pcm_free_chmaps(chmaps); > > - if (*found_chmap == NULL) { > + if (*found_chmap == NULL) > SNDERR("Found no matching channel map"); > - return -EINVAL; > - } > return 0; > } > > -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pcm: route: Don't handle no matching chmap as a serious error 2014-03-18 16:19 ` David Henningsson @ 2014-03-18 16:34 ` Takashi Iwai 2014-03-18 22:13 ` David Henningsson 0 siblings, 1 reply; 7+ messages in thread From: Takashi Iwai @ 2014-03-18 16:34 UTC (permalink / raw) To: David Henningsson; +Cc: alsa-devel At Tue, 18 Mar 2014 17:19:29 +0100, David Henningsson wrote: > > On 03/18/2014 03:27 PM, Takashi Iwai wrote: > > When find_matching_chmap() returns an error for the non-matching > > chmap, the caller, snd_pcm_route_open(), also returns an error > > although it shouldn't be handled as the fatal error. This results in > > the probe error with PulseAudio and it gives no real output in the > > end. > > Hmm, could you give a more specific example? In case the driver does not > support channel maps at all, that case is handled in the beginning of > the function. Well, the problem is that PulseAudio doesn't work at all with the current alsa-lib git prior to the commit in some cases. That is, the commit brought some incompatibility. > So this only happens if the driver supports channel maps, but only > non-compatible with the requested map. In which case I believe it's > correct that the probing should fail...? Could you check whether PA 5.0 works as is with alsa-lib git (before the last fix)? It could be seen on some desktop machines. Takashi > > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > --- > > src/pcm/pcm_route.c | 4 +--- > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c > > index ab17fa78be2c..ac11bdc8adfd 100644 > > --- a/src/pcm/pcm_route.c > > +++ b/src/pcm/pcm_route.c > > @@ -940,10 +940,8 @@ static int find_matching_chmap(snd_pcm_t *spcm, snd_pcm_chmap_t *tt_chmap, > > > > snd_pcm_free_chmaps(chmaps); > > > > - if (*found_chmap == NULL) { > > + if (*found_chmap == NULL) > > SNDERR("Found no matching channel map"); > > - return -EINVAL; > > - } > > return 0; > > } > > > > > > > > -- > David Henningsson, Canonical Ltd. > https://launchpad.net/~diwic > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pcm: route: Don't handle no matching chmap as a serious error 2014-03-18 16:34 ` Takashi Iwai @ 2014-03-18 22:13 ` David Henningsson 2014-03-19 9:57 ` Takashi Iwai 0 siblings, 1 reply; 7+ messages in thread From: David Henningsson @ 2014-03-18 22:13 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel [-- Attachment #1: Type: text/plain, Size: 2301 bytes --] On 03/18/2014 05:34 PM, Takashi Iwai wrote: > At Tue, 18 Mar 2014 17:19:29 +0100, > David Henningsson wrote: >> >> On 03/18/2014 03:27 PM, Takashi Iwai wrote: >>> When find_matching_chmap() returns an error for the non-matching >>> chmap, the caller, snd_pcm_route_open(), also returns an error >>> although it shouldn't be handled as the fatal error. This results in >>> the probe error with PulseAudio and it gives no real output in the >>> end. >> >> Hmm, could you give a more specific example? In case the driver does not >> support channel maps at all, that case is handled in the beginning of >> the function. > > Well, the problem is that PulseAudio doesn't work at all with the > current alsa-lib git prior to the commit in some cases. That is, the > commit brought some incompatibility. Hmm, then the error should be somewhere in determine_chmap instead. tt_chmap should be NULL (i e, find_matching_chmap is never called) for using the standard number syntax instead of the new chmap syntax. What about the patch attached instead (untested) ? >> So this only happens if the driver supports channel maps, but only >> non-compatible with the requested map. In which case I believe it's >> correct that the probing should fail...? > > Could you check whether PA 5.0 works as is with alsa-lib git (before > the last fix)? It could be seen on some desktop machines. Let me know if the attached patch works for you. In case it does not I will do this testing later. > > > Takashi > >> >>> >>> Signed-off-by: Takashi Iwai <tiwai@suse.de> >>> --- >>> src/pcm/pcm_route.c | 4 +--- >>> 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c >>> index ab17fa78be2c..ac11bdc8adfd 100644 >>> --- a/src/pcm/pcm_route.c >>> +++ b/src/pcm/pcm_route.c >>> @@ -940,10 +940,8 @@ static int find_matching_chmap(snd_pcm_t *spcm, snd_pcm_chmap_t *tt_chmap, >>> >>> snd_pcm_free_chmaps(chmaps); >>> >>> - if (*found_chmap == NULL) { >>> + if (*found_chmap == NULL) >>> SNDERR("Found no matching channel map"); >>> - return -EINVAL; >>> - } >>> return 0; >>> } >>> >>> >> >> >> >> -- >> David Henningsson, Canonical Ltd. >> https://launchpad.net/~diwic >> > -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic [-- Attachment #2: 0001-route-Return-NULL-in-case-of-zero-found-channels-in-.patch --] [-- Type: text/x-patch, Size: 850 bytes --] >From 746358818ead057a32330f7c8510cb65a7059e86 Mon Sep 17 00:00:00 2001 From: David Henningsson <david.henningsson@canonical.com> Date: Tue, 18 Mar 2014 23:07:19 +0100 Subject: [PATCH] route: Return NULL in case of zero found channels in determine_chmap This should fix the problem where the old route syntax can no longer be opened. Signed-off-by: David Henningsson <david.henningsson@canonical.com> --- src/pcm/pcm_route.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c index ac11bdc..a9097ca 100644 --- a/src/pcm/pcm_route.c +++ b/src/pcm/pcm_route.c @@ -883,7 +883,10 @@ static int determine_chmap(snd_config_t *tt, snd_pcm_chmap_t **tt_chmap) } } - + if (chmap->channels == 0) { + free(chmap); + chmap = NULL; + } *tt_chmap = chmap; return 0; -- 1.7.9.5 [-- Attachment #3: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] pcm: route: Don't handle no matching chmap as a serious error 2014-03-18 22:13 ` David Henningsson @ 2014-03-19 9:57 ` Takashi Iwai 2014-03-19 12:10 ` David Henningsson 0 siblings, 1 reply; 7+ messages in thread From: Takashi Iwai @ 2014-03-19 9:57 UTC (permalink / raw) To: David Henningsson; +Cc: alsa-devel At Tue, 18 Mar 2014 23:13:02 +0100, David Henningsson wrote: > > On 03/18/2014 05:34 PM, Takashi Iwai wrote: > > At Tue, 18 Mar 2014 17:19:29 +0100, > > David Henningsson wrote: > >> > >> On 03/18/2014 03:27 PM, Takashi Iwai wrote: > >>> When find_matching_chmap() returns an error for the non-matching > >>> chmap, the caller, snd_pcm_route_open(), also returns an error > >>> although it shouldn't be handled as the fatal error. This results in > >>> the probe error with PulseAudio and it gives no real output in the > >>> end. > >> > >> Hmm, could you give a more specific example? In case the driver does not > >> support channel maps at all, that case is handled in the beginning of > >> the function. > > > > Well, the problem is that PulseAudio doesn't work at all with the > > current alsa-lib git prior to the commit in some cases. That is, the > > commit brought some incompatibility. > > Hmm, then the error should be somewhere in determine_chmap instead. > tt_chmap should be NULL (i e, find_matching_chmap is never called) for > using the standard number syntax instead of the new chmap syntax. > > What about the patch attached instead (untested) ? > > >> So this only happens if the driver supports channel maps, but only > >> non-compatible with the requested map. In which case I believe it's > >> correct that the probing should fail...? > > > > Could you check whether PA 5.0 works as is with alsa-lib git (before > > the last fix)? It could be seen on some desktop machines. > > Let me know if the attached patch works for you. In case it does not I > will do this testing later. It seems working, but it revealed another bug, too. I fixed it in git tree, and reverted my last patch along with it. thanks, Takashi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pcm: route: Don't handle no matching chmap as a serious error 2014-03-19 9:57 ` Takashi Iwai @ 2014-03-19 12:10 ` David Henningsson 2014-03-19 13:17 ` Takashi Iwai 0 siblings, 1 reply; 7+ messages in thread From: David Henningsson @ 2014-03-19 12:10 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On 03/19/2014 10:57 AM, Takashi Iwai wrote: > At Tue, 18 Mar 2014 23:13:02 +0100, > David Henningsson wrote: >> >> On 03/18/2014 05:34 PM, Takashi Iwai wrote: >>> At Tue, 18 Mar 2014 17:19:29 +0100, >>> David Henningsson wrote: >>>> >>>> On 03/18/2014 03:27 PM, Takashi Iwai wrote: >>>>> When find_matching_chmap() returns an error for the non-matching >>>>> chmap, the caller, snd_pcm_route_open(), also returns an error >>>>> although it shouldn't be handled as the fatal error. This results in >>>>> the probe error with PulseAudio and it gives no real output in the >>>>> end. >>>> >>>> Hmm, could you give a more specific example? In case the driver does not >>>> support channel maps at all, that case is handled in the beginning of >>>> the function. >>> >>> Well, the problem is that PulseAudio doesn't work at all with the >>> current alsa-lib git prior to the commit in some cases. That is, the >>> commit brought some incompatibility. >> >> Hmm, then the error should be somewhere in determine_chmap instead. >> tt_chmap should be NULL (i e, find_matching_chmap is never called) for >> using the standard number syntax instead of the new chmap syntax. >> >> What about the patch attached instead (untested) ? >> >>>> So this only happens if the driver supports channel maps, but only >>>> non-compatible with the requested map. In which case I believe it's >>>> correct that the probing should fail...? >>> >>> Could you check whether PA 5.0 works as is with alsa-lib git (before >>> the last fix)? It could be seen on some desktop machines. >> >> Let me know if the attached patch works for you. In case it does not I >> will do this testing later. > > It seems working, but it revealed another bug, too. I fixed it in git > tree, and reverted my last patch along with it. I don't think the patch "route: Fix invalid pointer access" is necessary because determine_chmap is always called before any usage of tt_chmap, and the function always assigns a value to tt_chmap. But it does not hurt either, of course. Thanks for the testing! -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pcm: route: Don't handle no matching chmap as a serious error 2014-03-19 12:10 ` David Henningsson @ 2014-03-19 13:17 ` Takashi Iwai 0 siblings, 0 replies; 7+ messages in thread From: Takashi Iwai @ 2014-03-19 13:17 UTC (permalink / raw) To: David Henningsson; +Cc: alsa-devel At Wed, 19 Mar 2014 13:10:55 +0100, David Henningsson wrote: > > On 03/19/2014 10:57 AM, Takashi Iwai wrote: > > At Tue, 18 Mar 2014 23:13:02 +0100, > > David Henningsson wrote: > >> > >> On 03/18/2014 05:34 PM, Takashi Iwai wrote: > >>> At Tue, 18 Mar 2014 17:19:29 +0100, > >>> David Henningsson wrote: > >>>> > >>>> On 03/18/2014 03:27 PM, Takashi Iwai wrote: > >>>>> When find_matching_chmap() returns an error for the non-matching > >>>>> chmap, the caller, snd_pcm_route_open(), also returns an error > >>>>> although it shouldn't be handled as the fatal error. This results in > >>>>> the probe error with PulseAudio and it gives no real output in the > >>>>> end. > >>>> > >>>> Hmm, could you give a more specific example? In case the driver does not > >>>> support channel maps at all, that case is handled in the beginning of > >>>> the function. > >>> > >>> Well, the problem is that PulseAudio doesn't work at all with the > >>> current alsa-lib git prior to the commit in some cases. That is, the > >>> commit brought some incompatibility. > >> > >> Hmm, then the error should be somewhere in determine_chmap instead. > >> tt_chmap should be NULL (i e, find_matching_chmap is never called) for > >> using the standard number syntax instead of the new chmap syntax. > >> > >> What about the patch attached instead (untested) ? > >> > >>>> So this only happens if the driver supports channel maps, but only > >>>> non-compatible with the requested map. In which case I believe it's > >>>> correct that the probing should fail...? > >>> > >>> Could you check whether PA 5.0 works as is with alsa-lib git (before > >>> the last fix)? It could be seen on some desktop machines. > >> > >> Let me know if the attached patch works for you. In case it does not I > >> will do this testing later. > > > > It seems working, but it revealed another bug, too. I fixed it in git > > tree, and reverted my last patch along with it. > > I don't think the patch "route: Fix invalid pointer access" is necessary > because determine_chmap is always called before any usage of tt_chmap, > and the function always assigns a value to tt_chmap. It crashed in snd_pcm_route_close() before my fix. It called free() with an uninitialized pointer. Takashi ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-19 13:17 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-18 14:27 [PATCH] pcm: route: Don't handle no matching chmap as a serious error Takashi Iwai 2014-03-18 16:19 ` David Henningsson 2014-03-18 16:34 ` Takashi Iwai 2014-03-18 22:13 ` David Henningsson 2014-03-19 9:57 ` Takashi Iwai 2014-03-19 12:10 ` David Henningsson 2014-03-19 13:17 ` Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.