| Message ID | 1398941765-6593-1-git-send-email-linux@eikelenboom.it |
|---|---|
| State | New |
| Headers | show |
On 1 May 2014 11:56, Sander Eikelenboom <linux@eikelenboom.it> wrote: > Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it> > --- > configure | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/configure b/configure > index 2fbec59..de87d38 100755 > --- a/configure > +++ b/configure > @@ -637,6 +637,12 @@ Haiku) > ;; > *) > audio_drv_list="oss" > + if test -f /usr/include/sys/asoundlib.h ; then > + audio_drv_list="$audio_drv_list alsa" > + fi > + if test -f /usr/include/pulse/pulseaudio.h ; then > + audio_drv_list="$audio_drv_list pa" > + fi > audio_possible_drivers="oss alsa sdl esd pa" > linux="yes" > linux_user="yes" This is a lower-quality duplicate of the code we already have in configure for probing whether an audio backend is present (for instance it doesn't allow for the possibility that the headers have been installed elsewhere). It would be better to update the existing probe code to distinguish between "must be present", "don't use" and "use if present" for each audio driver, in the same way we do for other features. Then we could just set the default list here to "maybe-pa maybe-alsa oss" (syntax off the top of my head, feel free to have a better idea). thanks -- PMM
On 05/01/2014 04:56 AM, Sander Eikelenboom wrote: Your subject line is extremely long. A good commit has a one-line summary with 60 chars or less, then a blank line, then the details. Look at 'git shortlog -30' for a good idea of subject lines. > Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it> > --- > configure | 6 ++++++ > 1 file changed, 6 insertions(+) >
Thursday, May 1, 2014, 1:06:35 PM, you wrote: > On 1 May 2014 11:56, Sander Eikelenboom <linux@eikelenboom.it> wrote: >> Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it> >> --- >> configure | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/configure b/configure >> index 2fbec59..de87d38 100755 >> --- a/configure >> +++ b/configure >> @@ -637,6 +637,12 @@ Haiku) >> ;; >> *) >> audio_drv_list="oss" >> + if test -f /usr/include/sys/asoundlib.h ; then >> + audio_drv_list="$audio_drv_list alsa" >> + fi >> + if test -f /usr/include/pulse/pulseaudio.h ; then >> + audio_drv_list="$audio_drv_list pa" >> + fi >> audio_possible_drivers="oss alsa sdl esd pa" >> linux="yes" >> linux_user="yes" > This is a lower-quality duplicate of the code we already > have in configure for probing whether an audio backend > is present (for instance it doesn't allow for the possibility > that the headers have been installed elsewhere). It would > be better to update the existing probe code to distinguish > between "must be present", "don't use" and "use if present" > for each audio driver, in the same way we do for other > features. Then we could just set the default list here > to "maybe-pa maybe-alsa oss" (syntax off the top of my > head, feel free to have a better idea). How bad would it be for the generic targetos case to start with: audio_drv_list == audio_possible_drivers = "oss alsa sdl esd pa" or just add the probably most used two: audio_drv_list = "oss alsa pa" and just let the auto probing do it's business ? It would still be possible to overrule everything with an exact list by specifying: ./configure --audio_drv_list="something else" -- Sander > thanks > -- PMM
On 1 May 2014 13:56, Sander Eikelenboom <linux@eikelenboom.it> wrote: > Thursday, May 1, 2014, 1:06:35 PM, you wrote: >> This is a lower-quality duplicate of the code we already >> have in configure for probing whether an audio backend >> is present (for instance it doesn't allow for the possibility >> that the headers have been installed elsewhere). It would >> be better to update the existing probe code to distinguish >> between "must be present", "don't use" and "use if present" >> for each audio driver, in the same way we do for other >> features. Then we could just set the default list here >> to "maybe-pa maybe-alsa oss" (syntax off the top of my >> head, feel free to have a better idea). > > How bad would it be for the generic targetos case to start with: > > audio_drv_list == audio_possible_drivers = "oss alsa sdl esd pa" > > or just add the probably most used two: > audio_drv_list = "oss alsa pa" > > and just let the auto probing do it's business ? At the moment if you put something in audio_drv_list then configure will stop with an error if that driver can't be probed. So putting 'pa' or 'alsa' in the default list is a bad idea, because people whose setups worked fine for building qemu will suddenly find they stop working. We need both behaviours -- distributions want to be able to say "build with pulseaudio" and have the build fail if their pulseaudio dependencies aren't set up correctly rather than silently produce a package which won't work in their distro. But the average person building QEMU for their own use generally would prefer "probe for the features but if they're not present then just don't build the code that uses them". thanks -- PMM
Thursday, May 1, 2014, 3:06:27 PM, you wrote: > On 1 May 2014 13:56, Sander Eikelenboom <linux@eikelenboom.it> wrote: >> Thursday, May 1, 2014, 1:06:35 PM, you wrote: >>> This is a lower-quality duplicate of the code we already >>> have in configure for probing whether an audio backend >>> is present (for instance it doesn't allow for the possibility >>> that the headers have been installed elsewhere). It would >>> be better to update the existing probe code to distinguish >>> between "must be present", "don't use" and "use if present" >>> for each audio driver, in the same way we do for other >>> features. Then we could just set the default list here >>> to "maybe-pa maybe-alsa oss" (syntax off the top of my >>> head, feel free to have a better idea). >> >> How bad would it be for the generic targetos case to start with: >> >> audio_drv_list == audio_possible_drivers = "oss alsa sdl esd pa" >> >> or just add the probably most used two: >> audio_drv_list = "oss alsa pa" >> >> and just let the auto probing do it's business ? > At the moment if you put something in audio_drv_list > then configure will stop with an error if that driver > can't be probed. So putting 'pa' or 'alsa' in the > default list is a bad idea, because people whose setups > worked fine for building qemu will suddenly find they > stop working. Those thoughts only occur to me a few milliseconds after hitting the send button I think a "probe-" prefix would be more clear then the "maybe-" prefix. so "alsa" would require alsa and fail/exit the configure if the headers are not present "probe-alsa" would do the audio probe but not fail/exit the configure but just not select it for build. I will try brew something :-) > We need both behaviours -- distributions want to be able > to say "build with pulseaudio" and have the build fail > if their pulseaudio dependencies aren't set up correctly > rather than silently produce a package which won't work > in their distro. But the average person building QEMU > for their own use generally would prefer "probe for the > features but if they're not present then just don't > build the code that uses them". > thanks > -- PMM
diff --git a/configure b/configure index 2fbec59..de87d38 100755 --- a/configure +++ b/configure @@ -637,6 +637,12 @@ Haiku) ;; *) audio_drv_list="oss" + if test -f /usr/include/sys/asoundlib.h ; then + audio_drv_list="$audio_drv_list alsa" + fi + if test -f /usr/include/pulse/pulseaudio.h ; then + audio_drv_list="$audio_drv_list pa" + fi audio_possible_drivers="oss alsa sdl esd pa" linux="yes" linux_user="yes"
Signed-off-by: Sander Eikelenboom <linux@eikelenboom.it> --- configure | 6 ++++++ 1 file changed, 6 insertions(+)