diff mbox

Pulseaudio device removal

Message ID 4DFBB431.60101@redhat.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Mauro Carvalho Chehab June 17, 2011, 8:08 p.m. UTC
During a long time, the removal of an alsa drivers were a problem
for me, and other developers reported to have the same problem.

With Hans de Goede help, I've got the pulseaudio syntax that allows
releasing an alsa device. With that, I've added a patch to the media
build that will automatically handle it, when "make rmmod" is
called.

Feel free to test and give some feedback. I suspect that the logic
will need more hacks, as pulseaudio-libs is not capable of detecting
the name of some USB alsa drivers.

The logic there is not optimized: It is basically running pacmd list-sources
and pacmd list-sinks several times, as I didn't have time yet to optimize.
I'll probably do it soon, after finishing some other things.

Enjoy!
Mauro.


From 16cf0606fd59484236356e400a89c083e76da64b Mon Sep 17 00:00:00 2001
From: Mauro Carvalho Chehab <mchehab@redhat.com>
Date: Fri, 17 Jun 2011 16:48:04 -0300
Subject: [PATCH] rmmod.pl: Add a logic to allow removing audio modules with pulseaudio

Pulseaudio keeps audio devices opened forever. In order to be able to
remove a device, pulseaudio needs to de-allocate the device.

Unfortunately, pulseaudio recognizes alsa drivers as "module"
(an integer number, not related to the device nodename), and only
allows module removal if running with user matches the console owner.

The logic inside rmmod.pl will now take the above into account. So,
it will detect if pulseaudio is running. If it is, it will:

1) list the pulseaudio modules with "pacmd list-sinks"
   and "pacmd list-sources"

2) detect if any of the modules there is provided by a v4l device;

3) If they're provided by a video device, it removes the module with:
	pactl unload-module 26

Even the above logic is not perfect as, due to a pulseaudio libs bug,
pulseaudio can't detect the name of em28xx-alsa driver, as it uses the
same interface as the video node. Similar hacks will may be needed
for other USB devices, like tm6000 and cx231xx.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Jan Hoogenraad June 18, 2011, 5:20 p.m. UTC | #1
Mauro:

The change in build.sh
http://git.linuxtv.org/media_build.git?a=commitdiff;h=16cf0606fd59484236356e400a89c083e76da64b

now requires installation of a Perl package Proc::ProcessTable  that is 
not present in standard Ubuntu systems.

I needed to run
  sudo aptitude install libproc-processtable-perl
before I could continue after the change.

Is there a way around this ?


Mauro Carvalho Chehab wrote:
> During a long time, the removal of an alsa drivers were a problem
> for me, and other developers reported to have the same problem.
>
> With Hans de Goede help, I've got the pulseaudio syntax that allows
> releasing an alsa device. With that, I've added a patch to the media
> build that will automatically handle it, when "make rmmod" is
> called.
>
> Feel free to test and give some feedback. I suspect that the logic
> will need more hacks, as pulseaudio-libs is not capable of detecting
> the name of some USB alsa drivers.
>
> The logic there is not optimized: It is basically running pacmd list-sources
> and pacmd list-sinks several times, as I didn't have time yet to optimize.
> I'll probably do it soon, after finishing some other things.
>
> Enjoy!
> Mauro.
>
>
>> From 16cf0606fd59484236356e400a89c083e76da64b Mon Sep 17 00:00:00 2001
> From: Mauro Carvalho Chehab<mchehab@redhat.com>
> Date: Fri, 17 Jun 2011 16:48:04 -0300
> Subject: [PATCH] rmmod.pl: Add a logic to allow removing audio modules with pulseaudio
>
> Pulseaudio keeps audio devices opened forever. In order to be able to
> remove a device, pulseaudio needs to de-allocate the device.
>
> Unfortunately, pulseaudio recognizes alsa drivers as "module"
> (an integer number, not related to the device nodename), and only
> allows module removal if running with user matches the console owner.
>
> The logic inside rmmod.pl will now take the above into account. So,
> it will detect if pulseaudio is running. If it is, it will:
>
> 1) list the pulseaudio modules with "pacmd list-sinks"
>     and "pacmd list-sources"
>
> 2) detect if any of the modules there is provided by a v4l device;
>
> 3) If they're provided by a video device, it removes the module with:
> 	pactl unload-module 26
>
> Even the above logic is not perfect as, due to a pulseaudio libs bug,
> pulseaudio can't detect the name of em28xx-alsa driver, as it uses the
> same interface as the video node. Similar hacks will may be needed
> for other USB devices, like tm6000 and cx231xx.
>
> Signed-off-by: Mauro Carvalho Chehab<mchehab@redhat.com>
>
> diff --git a/build.sh b/build.sh
> index 60d0c26..3cc21f8 100755
> --- a/build.sh
> +++ b/build.sh
> @@ -12,6 +12,8 @@ echo "Checking if the needed tools are present"
>   run ./check_needs.pl
>   echo "Checking for Digest::SHA1 (package perl-Digest-SHA1)"
>   run perl -MDigest::SHA1 -e 1
> +echo "Checking for Proc::ProcessTable (package perl-Proc-ProcessTable)"
> +run perl -MProc::ProcessTable -e 1
>   echo
>   echo "************************************************************"
>   echo "* This script will download the latest tarball and build it*"
> diff --git a/v4l/scripts/rmmod.pl b/v4l/scripts/rmmod.pl
> index ed79cbe..53ea451 100755
> --- a/v4l/scripts/rmmod.pl
> +++ b/v4l/scripts/rmmod.pl
> @@ -1,6 +1,8 @@
>   #!/usr/bin/perl
>   use strict;
>   use File::Find;
> +use Proc::ProcessTable;
> +
>
>   my %depend = ();
>   my %depend2 = ();
> @@ -166,6 +168,69 @@ sub insmod ($)
>   	}
>   }
>
> +my @pulse;
> +my $try_pulseaudio = 1;
> +
> +sub check_pulseaudio()
> +{
> +	my $t = new Proc::ProcessTable;
> +	foreach my $p ( @{$t->table} ) {
> +		push @pulse, $p->uid if ($p->cmndline =~m,/pulseaudio ,);
> +	}
> +	$try_pulseaudio = 0 if (!@pulse);
> +
> +	print "Pulseaudio is running with UUID(s): @pulse\n";
> +}
> +
> +sub unload_pulseaudio($)
> +{
> +	my $driver_name = shift;
> +	my $cur_module;
> +
> +	return if (!$try_pulseaudio);
> +
> +	check_pulseaudio() if (!@pulse);
> +	return if (!$try_pulseaudio);
> +
> +	for my $pid (@pulse) {
> +#		printf "LANG=C sudo -u \\\#$pid pacmd list-sources |\n";
> +		open IN, "LANG=C sudo -u \\\#$pid pacmd list-sources |";
> +		while (<IN>) {
> +			$cur_module = $1 if (/^\s*module:\s*(\d+)/);
> +
> +			if (/^\s*alsa.driver_name\s*=\s*"(.*)"/) {
> +				if ($1 eq $driver_name) {
> +					print "LANG=C sudo -u \\#$pid pactl unload-module $cur_module\n";
> +					system ("LANG=C sudo -u \\#$pid pactl unload-module $cur_module");
> +				}
> +				next;
> +			}
> +
> +			# Special case: em28xx sometimes use a Vendor Class at
> +			# the same interface as the video node. Pulseaudio can't
> +			# get the driver name in this case
> +			if (/^\s*alsa.card_name\s*=\s*"Em28xx/) {
> +				print "LANG=C sudo -u \\#$pid pactl unload-module $cur_module\n";
> +				system ("LANG=C sudo -u \\#$pid pactl unload-module $cur_module");
> +			}
> +		}
> +		close IN;
> +
> +#		printf "LANG=C sudo -u \\\#$pid pacmd list-sinks |\n";
> +		open IN, "LANG=C sudo -u \\#$pid pacmd list-sinks |" or return;
> +		while (<IN>) {
> +			$cur_module = $1 if (/^\s*module:\s*(\d+)/);
> +			if (/^\s*alsa.driver_name\s*=\s*"(.*)"/) {
> +				if ($1 eq $driver_name) {
> +					print "LANG=C sudo -u \\#$pid pactl unload-module $1\n";
> +					system ("LANG=C sudo -u \\#$pid pactl unload-module $1");
> +				}
> +			}
> +		}
> +	}
> +	close IN;
> +}
> +
>   sub rmmod(@)
>   {
>   	my $rmmod = findprog('rmmod');
> @@ -173,8 +238,10 @@ sub rmmod(@)
>   	foreach (reverse @_) {
>   		s/-/_/g;
>   		if (exists ($loaded{$_})) {
> -			print "$rmmod $_\n";
> -			unshift @not, $_ if (system "$rmmod $_");
> +			my $module = $_;
> +			print "$rmmod $module\n";
> +			unload_pulseaudio($module);
> +			unshift @not, $module if (system "$rmmod $module");
>   		}
>   	}
>   	return @not;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Mauro Carvalho Chehab June 19, 2011, 12:15 p.m. UTC | #2
Em 18-06-2011 14:20, Jan Hoogenraad escreveu:
> Mauro:
> 
> The change in build.sh
> http://git.linuxtv.org/media_build.git?a=commitdiff;h=16cf0606fd59484236356e400a89c083e76da64b
> 
> now requires installation of a Perl package Proc::ProcessTable  that is not present in standard Ubuntu systems.
> 
> I needed to run
>  sudo aptitude install libproc-processtable-perl
> before I could continue after the change.
> 
> Is there a way around this ?

The media_build requires several packages that may not be present on some
installation. The build.sh script has a logic to detect the missing parts
and to output what's the missing requirements:

echo "Checking if the needed tools are present"
run ./check_needs.pl

(I moved right now the perl-specific checks into check_needs.pl script)

Unfortunately, package names are distro-specific. So, as I use only Fedora/RHEL
here, the only hints it have are for them. From my experiences, between the
rpm-based distros, the package names are either equal or very close, so such
hints probably are probably good enough for Suse and Mandriva users.

From what I understand, the standard Ubuntu repositories already provide a package
for Proc::ProcessTable. So, the only thing that it is not ok is the package name hint.

Could you please provide us a patch adding the Ubuntu (and likely Debian) package
name?

IMHO, the better would be to modify the check logic, in order to check what's the
system, and provide a hint based on it. If the OS type is not found, then fall back
to some default.

I think that the LSB default to get the distribution is by reading /etc/system-release.
Those are provided on RHEL6 and Fedora (plus, the old way: /etc/redhat-release).

So, IMHO, all we need to do is to write a logic for the error report part of the check,
that opens /etc/system-release, identify what's the distro, and provide the package
name and some instructions on how to install the missing parts to the userspace.

The right way for adding such logic would be to install the OS's on some VM with the
minimum install, run the script and add the missing parts on it.

Cheers,
Mauro.
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Hoogenraad June 19, 2011, 12:40 p.m. UTC | #3
Mauro:

You are completely right. Getting the packages automatically is very 
user-friendly. Furthermore, this will make media_build a great place to 
start of users stuck with older kernels.


On Ubuntu, the package name is  libproc-processtable-perl
the command to install it is (on Ubuntu, usually no root user is used, 
but rather per command invokation).

The command for installation will be:

  sudo apt-get install libproc-processtable-perl

There is no
/etc/system-release
on my system

However, there is a file /etc/lsb-release
cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=10.04
DISTRIB_CODENAME=lucid
DISTRIB_DESCRIPTION="Ubuntu 10.04.2 LTS"

I will make an updated script in the next week.

Mauro Carvalho Chehab wrote:
> Em 18-06-2011 14:20, Jan Hoogenraad escreveu:
>> Mauro:
>>
>> The change in build.sh
>> http://git.linuxtv.org/media_build.git?a=commitdiff;h=16cf0606fd59484236356e400a89c083e76da64b
>>
>> now requires installation of a Perl package Proc::ProcessTable  that is not present in standard Ubuntu systems.
>>
>> I needed to run
>>   sudo aptitude install libproc-processtable-perl
>> before I could continue after the change.
>>
>> Is there a way around this ?
>
> The media_build requires several packages that may not be present on some
> installation. The build.sh script has a logic to detect the missing parts
> and to output what's the missing requirements:
>
> echo "Checking if the needed tools are present"
> run ./check_needs.pl
>
> (I moved right now the perl-specific checks into check_needs.pl script)
>
> Unfortunately, package names are distro-specific. So, as I use only Fedora/RHEL
> here, the only hints it have are for them. From my experiences, between the
> rpm-based distros, the package names are either equal or very close, so such
> hints probably are probably good enough for Suse and Mandriva users.
>
>> From what I understand, the standard Ubuntu repositories already provide a package
> for Proc::ProcessTable. So, the only thing that it is not ok is the package name hint.
>
> Could you please provide us a patch adding the Ubuntu (and likely Debian) package
> name?
>
> IMHO, the better would be to modify the check logic, in order to check what's the
> system, and provide a hint based on it. If the OS type is not found, then fall back
> to some default.
>
> I think that the LSB default to get the distribution is by reading /etc/system-release.
> Those are provided on RHEL6 and Fedora (plus, the old way: /etc/redhat-release).
>
> So, IMHO, all we need to do is to write a logic for the error report part of the check,
> that opens /etc/system-release, identify what's the distro, and provide the package
> name and some instructions on how to install the missing parts to the userspace.
>
> The right way for adding such logic would be to install the OS's on some VM with the
> minimum install, run the script and add the missing parts on it.
>
> Cheers,
> Mauro.
>
Mauro Carvalho Chehab June 19, 2011, 12:50 p.m. UTC | #4
Em 19-06-2011 09:40, Jan Hoogenraad escreveu:
> Mauro:
> 
> You are completely right. Getting the packages automatically is very user-friendly. Furthermore, this will make media_build a great place to start of users stuck with older kernels.

Automatic install is good, but I think that providing a command line to the user
is better, as he may want to install things on a different way. For example, on
Mandriva, there are 2 or 3 different options to install package. I think that 
Debian/Ubuntu also provides at least 3 different ways (apt-get, aptitude, yum).

> 
> 
> On Ubuntu, the package name is  libproc-processtable-perl
> the command to install it is (on Ubuntu, usually no root user is used, but rather per command invokation).
> 
> The command for installation will be:
> 
>  sudo apt-get install libproc-processtable-perl
> 
> There is no
> /etc/system-release
> on my system
> 
> However, there is a file /etc/lsb-release
> cat /etc/lsb-release
> DISTRIB_ID=Ubuntu
> DISTRIB_RELEASE=10.04
> DISTRIB_CODENAME=lucid
> DISTRIB_DESCRIPTION="Ubuntu 10.04.2 LTS"
> 
> I will make an updated script in the next week.

OK. I just added a logic that works with RHEL/Fedora. There's no
/etc/lsb-release on RHEL 6.1 or Fedora 15, but it shouldn't be hard
to do support the Ubuntu way with:

 my $system_release = qx(cat /etc/system-release);
 $system_release = qx(cat /etc/redhat-release) if !$system_release;
+$system_release = qx(grep DISTRIB_ID /etc/lsb-release) if !$system_release;

Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/build.sh b/build.sh
index 60d0c26..3cc21f8 100755
--- a/build.sh
+++ b/build.sh
@@ -12,6 +12,8 @@  echo "Checking if the needed tools are present"
 run ./check_needs.pl
 echo "Checking for Digest::SHA1 (package perl-Digest-SHA1)"
 run perl -MDigest::SHA1 -e 1
+echo "Checking for Proc::ProcessTable (package perl-Proc-ProcessTable)"
+run perl -MProc::ProcessTable -e 1
 echo
 echo "************************************************************"
 echo "* This script will download the latest tarball and build it*"
diff --git a/v4l/scripts/rmmod.pl b/v4l/scripts/rmmod.pl
index ed79cbe..53ea451 100755
--- a/v4l/scripts/rmmod.pl
+++ b/v4l/scripts/rmmod.pl
@@ -1,6 +1,8 @@ 
 #!/usr/bin/perl
 use strict;
 use File::Find;
+use Proc::ProcessTable;
+
 
 my %depend = ();
 my %depend2 = ();
@@ -166,6 +168,69 @@  sub insmod ($)
 	}
 }
 
+my @pulse;
+my $try_pulseaudio = 1;
+
+sub check_pulseaudio()
+{
+	my $t = new Proc::ProcessTable;
+	foreach my $p ( @{$t->table} ) {
+		push @pulse, $p->uid if ($p->cmndline =~m,/pulseaudio ,);
+	}
+	$try_pulseaudio = 0 if (!@pulse);
+
+	print "Pulseaudio is running with UUID(s): @pulse\n";
+}
+
+sub unload_pulseaudio($)
+{
+	my $driver_name = shift;
+	my $cur_module;
+
+	return if (!$try_pulseaudio);
+
+	check_pulseaudio() if (!@pulse);
+	return if (!$try_pulseaudio);
+
+	for my $pid (@pulse) {
+#		printf "LANG=C sudo -u \\\#$pid pacmd list-sources |\n";
+		open IN, "LANG=C sudo -u \\\#$pid pacmd list-sources |";
+		while (<IN>) {
+			$cur_module = $1 if (/^\s*module:\s*(\d+)/);
+
+			if (/^\s*alsa.driver_name\s*=\s*"(.*)"/) {
+				if ($1 eq $driver_name) {
+					print "LANG=C sudo -u \\#$pid pactl unload-module $cur_module\n";
+					system ("LANG=C sudo -u \\#$pid pactl unload-module $cur_module");
+				}
+				next;
+			}
+
+			# Special case: em28xx sometimes use a Vendor Class at
+			# the same interface as the video node. Pulseaudio can't
+			# get the driver name in this case
+			if (/^\s*alsa.card_name\s*=\s*"Em28xx/) {
+				print "LANG=C sudo -u \\#$pid pactl unload-module $cur_module\n";
+				system ("LANG=C sudo -u \\#$pid pactl unload-module $cur_module");
+			}
+		}
+		close IN;
+
+#		printf "LANG=C sudo -u \\\#$pid pacmd list-sinks |\n";
+		open IN, "LANG=C sudo -u \\#$pid pacmd list-sinks |" or return;
+		while (<IN>) {
+			$cur_module = $1 if (/^\s*module:\s*(\d+)/);
+			if (/^\s*alsa.driver_name\s*=\s*"(.*)"/) {
+				if ($1 eq $driver_name) {
+					print "LANG=C sudo -u \\#$pid pactl unload-module $1\n";
+					system ("LANG=C sudo -u \\#$pid pactl unload-module $1");
+				}
+			}
+		}
+	}
+	close IN;
+}
+
 sub rmmod(@)
 {
 	my $rmmod = findprog('rmmod');
@@ -173,8 +238,10 @@  sub rmmod(@)
 	foreach (reverse @_) {
 		s/-/_/g;
 		if (exists ($loaded{$_})) {
-			print "$rmmod $_\n";
-			unshift @not, $_ if (system "$rmmod $_");
+			my $module = $_;
+			print "$rmmod $module\n";
+			unload_pulseaudio($module);
+			unshift @not, $module if (system "$rmmod $module");
 		}
 	}
 	return @not;