Re: [pulseaudio-discuss] [PATCH 0/5] Reconfigure sample rates on resume

2011-03-08 Thread Arun Raghavan 
On Sun, 2011-03-06 at 16:30 -0600, Pierre-Louis Bossart wrote:
 This is the second version of my endeavors to remove or make SRC
 simpler when possible. An idle non-network, local or passthrough
 devices will be suspended immediately when there aren't any
 inputs. When a new input is connected, we try to resume with the best match
 between requested sample rate and sink sample rate. To make things
 simple, the sink can only support a default and an alternate rate,
 typically 44.1 and 48kHz. Of course the feature is disabled when both
 rates are identical (if the sink cannot be reconfigured).  Tests with
 HDAudio and USB show a negligible added delay but a nice reduction of
 CPU activity.

An alternative idea:

At sink-input trigger a reconfiguration if possible/supported. Sinks
expose a reconfigure_rate method (I'm sure we can find a better name) if
they can do a quick switch.

In the ALSA sink, this function would check if there are no sink-inputs
attached and the rate is supported, and then send a message to the I/O
thread to do the actual reconfiguration. We would cache the rate if
suspended and apply on unsuspend, or do a suspend-reconfigure-unsuspend
if it's running.

With this, we can replace default-sample-rate with a minimum-sample-rate
(anything below this gets upsampled), and use the sink-input sample rate
if supported (or an integer multiple as your code does). The advantage
of course is that we're no longer restricted to two sample rates.

I did a quick walk over the code and this seems feasible. I'm doing
something similar (but far simpler) for passthrough mode in alsa-sink -
when a passthrough input is added, I do the
suspend-update_rate-unsuspend if required and it seems to work fine.

Does this make sense or am I missing something basic here?

Cheers,
Arun

___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] Conexant CX20585 recording does not work

2011-03-08 Thread Colin Guthrie 
'Twas brillig, and David Henningsson at 08/03/11 02:44 did gyre and gimble:
 This one is one of few that lack an auto-parser of the pin default
 config, so for now - model quirks. Without seeing alsa-info or something
 similar, I can't tell which model would fit.
 
 In the long run we would benefit from an autoparser handling these as well.

OK, so Gabriel, this sounds like one to take over to alsa-devel mailing
list (I'd avoid their bug tracker - it's a black hole!). You should
attach the alsa-info output to that mail.

Feel free to CC me (and maybe David too if he doesn't object!).

David has a good alsa-info.sh howto here:
https://wiki.ubuntu.com/Audio/AlsaInfo

Cheers

Col

-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mageia Contributor [http://www.mageia.org/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]

___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [RFC PATCH] Profile for Nvidia

2011-03-08 Thread David Henningsson 
This patch is RFC both because I haven't tested it properly yet 
(although I expect it to work!) but mainly because I'm uncertain of what 
surround profiles we should add for HDMI. Opinions about that or other 
patches?


I don't think it relates to Kelly's patch posted earlier this week or 
other pass-through patches.


--
David Henningsson, Canonical Ltd.
http://launchpad.net/~diwic
From 8438e14d1101f195e4c1a5dff41fdbacbc9366df Mon Sep 17 00:00:00 2001
From: David Henningsson david.hennings...@canonical.com
Date: Tue, 8 Mar 2011 10:51:33 +0100
Subject: [PATCH] alsa-mixer: Add separate profile for Nvidia

This is a profile for all Nvidia cards - some Nvidia cards have four HDMI codecs,
and which ones are working seems to vary a lot between GPU boards. In addition,
Nvidia makes southbridges as well, so we need to keep the existing analog profiles.
(And by not adding all these extra profiles to default.conf, we make sure
there is no performance hit for non-Nvidia cards.)

Once we have proper jack detection, there would probably be room for improvement here.

Signed-off-by: David Henningsson david.hennings...@canonical.com
---
 src/Makefile.am|1 +
 .../alsa/mixer/profile-sets/90-pulseaudio.rules|1 +
 src/modules/alsa/mixer/profile-sets/nvidia.conf|  160 
 3 files changed, 162 insertions(+), 0 deletions(-)
 create mode 100644 src/modules/alsa/mixer/profile-sets/nvidia.conf

diff --git a/src/Makefile.am b/src/Makefile.am
index ec301da..4fc68a6 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -109,6 +109,7 @@ MODULE_LDFLAGS = -module -disable-static -avoid-version $(LDFLAGS_NOUNDEFINED)
 
 ALSA_PROFILES = \
 		modules/alsa/mixer/profile-sets/default.conf \
+		modules/alsa/mixer/profile-sets/nvidia.conf \
 		modules/alsa/mixer/profile-sets/maudio-fasttrack-pro.conf \
 		modules/alsa/mixer/profile-sets/native-instruments-audio4dj.conf \
 		modules/alsa/mixer/profile-sets/native-instruments-audio8dj.conf \
diff --git a/src/modules/alsa/mixer/profile-sets/90-pulseaudio.rules b/src/modules/alsa/mixer/profile-sets/90-pulseaudio.rules
index f964b00..3b7078f 100644
--- a/src/modules/alsa/mixer/profile-sets/90-pulseaudio.rules
+++ b/src/modules/alsa/mixer/profile-sets/90-pulseaudio.rules
@@ -25,5 +25,6 @@ SUBSYSTEMS==usb, ATTRS{idVendor}==17cc, ATTRS{idProduct}==0839, ENV{PULSE_
 SUBSYSTEMS==usb, ATTRS{idVendor}==17cc, ATTRS{idProduct}==baff, ENV{PULSE_PROFILE_SET}=native-instruments-traktorkontrol-s4.conf
 SUBSYSTEMS==usb, ATTRS{idVendor}==17cc, ATTRS{idProduct}==4711, ENV{PULSE_PROFILE_SET}=native-instruments-korecontroller.conf
 SUBSYSTEMS==usb, ATTRS{idVendor}==0763, ATTRS{idProduct}==2012, ENV{PULSE_PROFILE_SET}=maudio-fasttrack-pro.conf
+ATTRS{idVendor}==10de, ENV{PULSE_PROFILE_SET}=nvidia.conf
 
 LABEL=pulseaudio_end
diff --git a/src/modules/alsa/mixer/profile-sets/nvidia.conf b/src/modules/alsa/mixer/profile-sets/nvidia.conf
new file mode 100644
index 000..69a371f
--- /dev/null
+++ b/src/modules/alsa/mixer/profile-sets/nvidia.conf
@@ -0,0 +1,160 @@
+# This file is part of PulseAudio.
+#
+# PulseAudio is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as
+# published by the Free Software Foundation; either version 2.1 of the
+# License, or (at your option) any later version.
+#
+# PulseAudio 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 Lesser General Public License
+# along with PulseAudio; if not, write to the Free Software Foundation,
+# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+
+; This is a profile for all Nvidia cards - some Nvidia cards have four HDMI codecs,
+; and which ones are working seems to vary a lot between GPU boards. In addition,
+; Nvidia makes southbridges as well, so we need to keep the existing analog profiles.
+; (And by not adding all these extra profiles to default.conf, we make sure
+; there is no performance hit for non-Nvidia cards.)
+
+; Once we have proper jack detection, there would probably be room for improvement here.
+
+[General]
+auto-profiles = yes
+
+[Mapping analog-mono]
+device-strings = hw:%f
+channel-map = mono
+paths-output = analog-output analog-output-speaker analog-output-desktop-speaker analog-output-headphones analog-output-headphones-2 analog-output-mono analog-output-lfe-on-mono
+paths-input = analog-input analog-input-mic analog-input-linein analog-input-aux analog-input-video analog-input-tvtuner analog-input-fm analog-input-mic-line
+priority = 1
+
+[Mapping analog-stereo]
+device-strings = front:%f hw:%f
+channel-map = left,right
+paths-output = analog-output analog-output-speaker analog-output-desktop-speaker analog-output-headphones analog-output-headphones-2

Re: [pulseaudio-discuss] [PATCH 0/5] Reconfigure sample rates on resume

2011-03-08 Thread pl bossart 
 At sink-input trigger a reconfiguration if possible/supported. Sinks
 expose a reconfigure_rate method (I'm sure we can find a better name) if
 they can do a quick switch.

So far it's similar to the update_rate() method I added on the sink,

 In the ALSA sink, this function would check if there are no sink-inputs
 attached and the rate is supported, and then send a message to the I/O
 thread to do the actual reconfiguration. We would cache the rate if
 suspended and apply on unsuspend, or do a suspend-reconfigure-unsuspend
 if it's running.

It's interesting.
The only problems I see is a potential duplication of code, since you
force the suspend in the sink, and it's already part of
module-suspend-on-idle. Meaning you will need to implement this
message in the BT sink as well. I wonder if there would be potential
races if the suspend-on-idle timeout happens right after you enable
the new stream.

 With this, we can replace default-sample-rate with a minimum-sample-rate
 (anything below this gets upsampled), and use the sink-input sample rate
 if supported (or an integer multiple as your code does). The advantage
 of course is that we're no longer restricted to two sample rates.

I don't really see a benefit of having more than 2 sample rates. The
only exception is
passthrough, where you want to forward the sink-input frequency as is.
And reading again I think it's the same behavior I implemented?

 I did a quick walk over the code and this seems feasible. I'm doing
 something similar (but far simpler) for passthrough mode in alsa-sink -
 when a passthrough input is added, I do the
 suspend-update_rate-unsuspend if required and it seems to work fine.

 Does this make sense or am I missing something basic here?

Couldn't you add the same behavior with my patches? If the sink-input
is passthrough then the desired rate is the sink-input rate.
Overall I am still not clear on what your alternative implementation
brings? But I am badly jet-lagged, I may have missed something as
well.
-Pierre
___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] [PATCH 0/5] Reconfigure sample rates on resume

2011-03-08 Thread Arun Raghavan 
On Tue, 2011-03-08 at 07:03 -0600, pl bossart wrote:
  At sink-input trigger a reconfiguration if possible/supported. Sinks
  expose a reconfigure_rate method (I'm sure we can find a better name) if
  they can do a quick switch.
 
 So far it's similar to the update_rate() method I added on the sink,
  In the ALSA sink, this function would check if there are no sink-inputs
  attached and the rate is supported, and then send a message to the I/O
  thread to do the actual reconfiguration. We would cache the rate if
  suspended and apply on unsuspend, or do a suspend-reconfigure-unsuspend
  if it's running.
 
 It's interesting.
 The only problems I see is a potential duplication of code, since you
 force the suspend in the sink, and it's already part of
 module-suspend-on-idle. Meaning you will need to implement this
 message in the BT sink as well. I wonder if there would be potential

I assumed we'd only be implementing this in ALSA for now since we're not
allowing reconfiguration on BT in your patches either. Either way, I
don't expect there to be too much duplication.

 races if the suspend-on-idle timeout happens right after you enable
 the new stream.

This should get serialised in the mainloop as far as I can tell.

  With this, we can replace default-sample-rate with a minimum-sample-rate
  (anything below this gets upsampled), and use the sink-input sample rate
  if supported (or an integer multiple as your code does). The advantage
  of course is that we're no longer restricted to two sample rates.
 
 I don't really see a benefit of having more than 2 sample rates. The
 only exception is
 passthrough, where you want to forward the sink-input frequency as is.
 And reading again I think it's the same behavior I implemented?

The benefit is just that if you have ultra-high-quality 88.4/96 kHz
music, it doesn't get resampled. I honestly don't know how many people
out there care about this, but if the hardware supports higher sample
rates, I see no reason to not expose the ability to play that.

  I did a quick walk over the code and this seems feasible. I'm doing
  something similar (but far simpler) for passthrough mode in alsa-sink -
  when a passthrough input is added, I do the
  suspend-update_rate-unsuspend if required and it seems to work fine.
 
  Does this make sense or am I missing something basic here?
 
 Couldn't you add the same behavior with my patches? If the sink-input
 is passthrough then the desired rate is the sink-input rate.
 Overall I am still not clear on what your alternative implementation
 brings? But I am badly jet-lagged, I may have missed something as
 well.

Yes, whatever the final solution is, I'll definitely be reusing this API
in my passthrough work.

Cheers,
Arun

___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [PATCH 0/2] Log : Add a new log target to a file descriptor

2011-03-08 Thread Vincent Becker 
These 2 patches are adding a log target to Pulseaudio. This new log target is a 
file descriptor (regular file, char device...). It also reformats the log 
message for optimal calls to the write function.

Vincent Becker (2):
  Log feature:Add a new log target to a file descriptor
  Log feature: optimize log data format before writing to target

 src/daemon/cmdline.c |5 +-
 src/daemon/daemon-conf.c |   34 ++-
 src/pulsecore/log.c  |  154 +++---
 src/pulsecore/log.h  |5 ++
 4 files changed, 144 insertions(+), 54 deletions(-)

-- 
1.7.2.3

-
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: Les Montalets- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [PATCH 1/2] Log feature:Add a new log target to a file descriptor

2011-03-08 Thread Vincent Becker 
This patch enables logging of text debug messages (pa_log feature) into a file 
or a device driver.
Example : pulseaudio --log-target=file:./mylog.txt

Signed-off-by: Vincent Becker vincentx.bec...@intel.com
---
 src/daemon/cmdline.c |5 +++--
 src/daemon/daemon-conf.c |   34 --
 src/pulsecore/log.c  |   24 
 src/pulsecore/log.h  |5 +
 4 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/src/daemon/cmdline.c b/src/daemon/cmdline.c
index f6cdcdc..2c3eb67 100644
--- a/src/daemon/cmdline.c
+++ b/src/daemon/cmdline.c
@@ -145,7 +145,8 @@ void pa_cmdline_help(const char *argv0) {
this time passed\n
  --log-level[=LEVEL]   Increase or set verbosity 
level\n
  -vIncrease the verbosity 
level\n
- --log-target={auto,syslog,stderr} Specify the log target\n
+ --log-target={auto,syslog,stderr,\n
+   file:PATH}  Specify the log target\n
  --log-meta[=BOOL] Include code location in 
log messages\n
  --log-time[=BOOL] Include timestamps in log 
messages\n
  --log-backtrace=FRAMESInclude a backtrace in log 
messages\n
@@ -318,7 +319,7 @@ int pa_cmdline_parse(pa_daemon_conf *conf, int argc, char 
*const argv [], int *d
 
 case ARG_LOG_TARGET:
 if (pa_daemon_conf_set_log_target(conf, optarg)  0) {
-pa_log(_(Invalid log target: use either 'syslog', 
'stderr' or 'auto'.));
+pa_log(_(Invalid log target: use either 'syslog', 
'stderr', 'auto' or a valid file name 'file:path'.));
 goto fail;
 }
 break;
diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
index e38e67a..5696f00 100644
--- a/src/daemon/daemon-conf.c
+++ b/src/daemon/daemon-conf.c
@@ -28,6 +28,8 @@
 #include stdio.h
 #include string.h
 #include unistd.h
+#include sys/stat.h
+#include fcntl.h
 
 #ifdef HAVE_SCHED_H
 #include sched.h
@@ -141,6 +143,9 @@ static const pa_daemon_conf default_conf = {
 #endif
 };
 
+static int log_fd = -1;
+
+
 pa_daemon_conf* pa_daemon_conf_new(void) {
 pa_daemon_conf *c;
 
@@ -166,6 +171,9 @@ pa_daemon_conf* pa_daemon_conf_new(void) {
 
 void pa_daemon_conf_free(pa_daemon_conf *c) {
 pa_assert(c);
+if (log_fd = 0)
+pa_close(log_fd);
+
 pa_xfree(c-script_commands);
 pa_xfree(c-dl_search_path);
 pa_xfree(c-default_script_file);
@@ -185,8 +193,30 @@ int pa_daemon_conf_set_log_target(pa_daemon_conf *c, const 
char *string) {
 } else if (!strcmp(string, stderr)) {
 c-auto_log_target = 0;
 c-log_target = PA_LOG_STDERR;
-} else
-return -1;
+} else if (pa_startswith(string, file:)) {
+char *file_path;
+const char *state = NULL;
+unsigned i;
+
+/* After second pa_split call, file_path will contain the right part 
of file:path */
+for (i = 0; i  2; i++)
+file_path = pa_split(string, :, state);
+
+/* Check if the file is regular */
+if ((log_fd = open(file_path, O_RDWR|O_TRUNC|O_CREAT, 0777)) = 0) {
+c-auto_log_target = 0;
+c-log_target = PA_LOG_FD;
+pa_log_set_fd(log_fd);
+}
+else {
+printf(Failed to open target file %s, error : %s\n, file_path, 
pa_cstrerror(errno));
+pa_xfree(file_path);
+return -1;
+}
+pa_xfree(file_path);
+}
+else
+  return -1;
 
 return 0;
 }
diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
index 2c0e267..a5e26c8 100644
--- a/src/pulsecore/log.c
+++ b/src/pulsecore/log.c
@@ -71,6 +71,8 @@ static unsigned show_backtrace = 0, show_backtrace_override = 
0, skip_backtrace
 static pa_log_flags_t flags = 0, flags_override = 0;
 static pa_bool_t no_rate_limit = FALSE;
 
+static int fdlog = -1;
+
 #ifdef HAVE_SYSLOG_H
 static const int level_to_syslog[] = {
 [PA_LOG_ERROR] = LOG_ERR,
@@ -128,6 +130,12 @@ void pa_log_set_flags(pa_log_flags_t _flags, 
pa_log_merge_t merge) {
 flags = _flags;
 }
 
+void pa_log_set_fd(int fd) {
+pa_assert(fd = 0);
+
+fdlog = fd;
+}
+
 void pa_log_set_show_backtrace(unsigned nlevels) {
 show_backtrace = nlevels;
 }
@@ -399,6 +407,22 @@ void pa_log_levelv_meta(
 }
 #endif
 
+case PA_LOG_FD: {
+if (fdlog != -1) {
+char metadata[256];
+
+pa_snprintf(metadata, sizeof(metadata), \n%c %s%s, 
level_to_char[level], timestamp, location);
+
+if ((write(fdlog, metadata, strlen(metadata))  0) || 
(write(fdlog, t, strlen(t))  0)) {
+saved_errno = errno;
+pa_close(fdlog);
+

[pulseaudio-discuss] [PATCH 2/2] Log feature: optimize log data format before writing to target

2011-03-08 Thread Vincent Becker 
Format data appended and prepended to the log message so that only one call to 
write is necessary

Signed-off-by: Vincent Becker vincentx.bec...@intel.com
---
 src/pulsecore/log.c |  160 ++-
 1 files changed, 95 insertions(+), 65 deletions(-)

diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
index a5e26c8..abb9066 100644
--- a/src/pulsecore/log.c
+++ b/src/pulsecore/log.c
@@ -277,10 +277,12 @@ void pa_log_levelv_meta(
 pa_log_level_t _maximum_level;
 unsigned _show_backtrace;
 pa_log_flags_t _flags;
+char *local_t;
+int metadata_length;
 
 /* We don't use dynamic memory allocation here to minimize the hit
  * in RT threads */
-char text[16*1024], location[128], timestamp[32];
+char text[16*1024], location[128], timestamp[32], metadata[256], 
append_data[256];
 
 pa_assert(level  PA_LOG_LEVEL_MAX);
 pa_assert(format);
@@ -297,8 +299,6 @@ void pa_log_levelv_meta(
 return;
 }
 
-pa_vsnprintf(text, sizeof(text), format, ap);
-
 if ((_flags  PA_LOG_PRINT_META)  file  line  0  func)
 pa_snprintf(location, sizeof(location), [%s:%i %s()] , file, line, 
func);
 else if ((_flags  (PA_LOG_PRINT_META|PA_LOG_PRINT_FILE))  file)
@@ -338,90 +338,120 @@ void pa_log_levelv_meta(
 bt = get_backtrace(_show_backtrace);
 #endif
 
-if (!pa_utf8_valid(text))
+switch (_target) {
+
+case PA_LOG_STDERR: {
+const char *prefix = , *suffix = , *grey = ;
+
+#ifndef OS_IS_WIN32
+   /* Yes indeed. Useless, but fun! */
+   if ((_flags  PA_LOG_COLORS)  isatty(STDERR_FILENO)) {
+   if (level = PA_LOG_ERROR)
+   prefix = \x1B[1;31m;
+   else if (level = PA_LOG_WARN)
+   prefix = \x1B[1m;
+
+   if (bt)
+   grey = \x1B[2m;
+
+   if (grey[0] || prefix[0])
+   suffix = \x1B[0m;
+   }
+#endif
+if (_flags  PA_LOG_PRINT_LEVEL)
+pa_snprintf(metadata, sizeof(metadata), %s%c: %s%s, timestamp, 
level_to_char[level], location, prefix);
+   else
+pa_snprintf(metadata, sizeof(metadata), %s%s%s, timestamp, 
location, prefix);
+
+   pa_snprintf(append_data, sizeof(append_data), %s%s%s, grey, 
pa_strempty(bt), suffix);
+
+   break;
+}
+
+#ifdef HAVE_SYSLOG_H
+case PA_LOG_SYSLOG: {
+pa_snprintf(metadata, sizeof(metadata), %s%s, timestamp, location);
+   pa_snprintf(append_data, sizeof(append_data) ,%s, pa_strempty(bt));
+
+break;
+}
+#endif
+
+case PA_LOG_FD: {
+   pa_snprintf(metadata, sizeof(metadata), \n%c %s%s, 
level_to_char[level], timestamp, location);
+
+append_data[0] = 0;
+
+break;
+}
+
+default:
+metadata[0] = 0;
+   append_data[0] = 0;
+
+break;
+}
+
+metadata_length = strlen(metadata);
+
+/* Format text at metadata_length position */
+pa_vsnprintf(text[metadata_length], sizeof(text) - metadata_length, 
format, ap);
+
+if (!pa_utf8_valid(text[metadata_length]))
 pa_logl(level, Invalid UTF-8 string following below:);
 
-for (t = text; t; t = n) {
-if ((n = strchr(t, '\n'))) {
-*n = 0;
-n++;
-}
+/* We shouldn't be using dynamic allocation here to
+ * minimize the hit in RT threads */
+if ((local_t = pa_utf8_to_locale(text[metadata_length]))) {
+   strcpy(text[metadata_length], local_t);
+   pa_xfree(local_t);
+}
+
+for (t = text[metadata_length]; t; t = n) {
+   if ((n = strchr(t, '\n'))) {
+   *n = 0;
+   n++;
+   }
 
 /* We ignore strings only made out of whitespace */
 if (t[strspn(t, \t )] == 0)
 continue;
 
-switch (_target) {
+   /* Rewind to the start of the t buffer */
+   t -= metadata_length;
 
-case PA_LOG_STDERR: {
-const char *prefix = , *suffix = , *grey = ;
-char *local_t;
+   /* Prepend metadata into t */
+   memcpy(t, metadata, metadata_length);
 
-#ifndef OS_IS_WIN32
-/* Yes indeed. Useless, but fun! */
-if ((_flags  PA_LOG_COLORS)  isatty(STDERR_FILENO)) {
-if (level = PA_LOG_ERROR)
-prefix = \x1B[1;31m;
-else if (level = PA_LOG_WARN)
-prefix = \x1B[1m;
-
-if (bt)
-grey = \x1B[2m;
-
-if (grey[0] || prefix[0])
-suffix = \x1B[0m;
-}
-#endif
-
-/* We shouldn't be using dynamic allocation here to
- * minimize the hit in RT threads */
-if ((local_t = pa_utf8_to_locale(t)))
-t = local_t;
-
-if (_flags  PA_LOG_PRINT_LEVEL)
-fprintf(stderr, %s%c: %s%s%s%s%s%s\n, timestamp, 
level_to_char[level], location, prefix, t, grey,

Re: [pulseaudio-discuss] [PATCH 2/2] Log feature: optimize log data format before writing to target

2011-03-08 Thread Becker, VincentX 
-Original Message-
From: pulseaudio-discuss-boun...@mail.0pointer.de [mailto:pulseaudio-
discuss-boun...@mail.0pointer.de] On Behalf Of Vincent Becker
Sent: Tuesday, March 08, 2011 5:15 PM
To: pulseaudio-discuss@mail.0pointer.de
Subject: [pulseaudio-discuss] [PATCH 2/2] Log feature: optimize log data
format before writing to target


Note that this patch would need to be discussed on the list. Its main purpose 
is to avoid multiple calls to write, which can be the case when writing to a 
file descriptor. Indeed, the patch applied before did 2 calls to write as the 
metadata needed to be formatted in a new buffer and written apart of the text 
log message. Making only 1 call to write instead of 2 can be precious in 
embedded environments. It was easier and clearer to apply this change to all 
the log targets : 'char *t' now contains in order 'metadata - text - 
append_data' (append_data is not used in every case though). It also has the 
advantage to clarify a bit which data in which case is logged..

Vincent


Format data appended and prepended to the log message so that only one
call to write is necessary

Signed-off-by: Vincent Becker vincentx.bec...@intel.com
---
 src/pulsecore/log.c |  160 ++--
---
 1 files changed, 95 insertions(+), 65 deletions(-)

diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
index a5e26c8..abb9066 100644
--- a/src/pulsecore/log.c
+++ b/src/pulsecore/log.c
@@ -277,10 +277,12 @@ void pa_log_levelv_meta(
 pa_log_level_t _maximum_level;
 unsigned _show_backtrace;
 pa_log_flags_t _flags;
+char *local_t;
+int metadata_length;

 /* We don't use dynamic memory allocation here to minimize the hit
  * in RT threads */
-char text[16*1024], location[128], timestamp[32];
+char text[16*1024], location[128], timestamp[32], metadata[256],
append_data[256];

 pa_assert(level  PA_LOG_LEVEL_MAX);
 pa_assert(format);
@@ -297,8 +299,6 @@ void pa_log_levelv_meta(
 return;
 }

-pa_vsnprintf(text, sizeof(text), format, ap);
-
 if ((_flags  PA_LOG_PRINT_META)  file  line  0  func)
 pa_snprintf(location, sizeof(location), [%s:%i %s()] , file,
line, func);
 else if ((_flags  (PA_LOG_PRINT_META|PA_LOG_PRINT_FILE))  file)
@@ -338,90 +338,120 @@ void pa_log_levelv_meta(
 bt = get_backtrace(_show_backtrace);
 #endif

-if (!pa_utf8_valid(text))
+switch (_target) {
+
+case PA_LOG_STDERR: {
+const char *prefix = , *suffix = , *grey = ;
+
+#ifndef OS_IS_WIN32
+  /* Yes indeed. Useless, but fun! */
+  if ((_flags  PA_LOG_COLORS)  isatty(STDERR_FILENO)) {
+  if (level = PA_LOG_ERROR)
+  prefix = \x1B[1;31m;
+  else if (level = PA_LOG_WARN)
+  prefix = \x1B[1m;
+
+  if (bt)
+  grey = \x1B[2m;
+
+  if (grey[0] || prefix[0])
+  suffix = \x1B[0m;
+  }
+#endif
+if (_flags  PA_LOG_PRINT_LEVEL)
+pa_snprintf(metadata, sizeof(metadata), %s%c: %s%s,
timestamp, level_to_char[level], location, prefix);
+  else
+pa_snprintf(metadata, sizeof(metadata), %s%s%s,
timestamp, location, prefix);
+
+  pa_snprintf(append_data, sizeof(append_data), %s%s%s, grey,
pa_strempty(bt), suffix);
+
+  break;
+}
+
+#ifdef HAVE_SYSLOG_H
+case PA_LOG_SYSLOG: {
+pa_snprintf(metadata, sizeof(metadata), %s%s, timestamp,
location);
+  pa_snprintf(append_data, sizeof(append_data) ,%s,
pa_strempty(bt));
+
+break;
+}
+#endif
+
+case PA_LOG_FD: {
+  pa_snprintf(metadata, sizeof(metadata), \n%c %s%s,
level_to_char[level], timestamp, location);
+
+append_data[0] = 0;
+
+break;
+}
+
+default:
+metadata[0] = 0;
+  append_data[0] = 0;
+
+break;
+}
+
+metadata_length = strlen(metadata);
+
+/* Format text at metadata_length position */
+pa_vsnprintf(text[metadata_length], sizeof(text) -
metadata_length, format, ap);
+
+if (!pa_utf8_valid(text[metadata_length]))
 pa_logl(level, Invalid UTF-8 string following below:);

-for (t = text; t; t = n) {
-if ((n = strchr(t, '\n'))) {
-*n = 0;
-n++;
-}
+/* We shouldn't be using dynamic allocation here to
+ * minimize the hit in RT threads */
+if ((local_t = pa_utf8_to_locale(text[metadata_length]))) {
+  strcpy(text[metadata_length], local_t);
+  pa_xfree(local_t);
+}
+
+for (t = text[metadata_length]; t; t = n) {
+  if ((n = strchr(t, '\n'))) {
+  *n = 0;
+  n++;
+  }

 /* We ignore strings only made out of whitespace */
 if (t[strspn(t, \t )] == 0)
 continue;

-switch (_target) {
+  /* Rewind to the start of the t buffer */
+  t -= metadata_length;

-case PA_LOG_STDERR: {
-const char *prefix = , *suffix = , *grey = ;
-char *local_t;
+

Re: [pulseaudio-discuss] [PATCH 1/2] Log feature:Add a new log target to a file descriptor

2011-03-08 Thread Maarten Bosmans 
FWIW, I've tested this patch and it works as advertised.

As we already decided in the IRC meeting that the concept of the patch
was fine, this is ready to go in git master. (with some minor cosmetic
adjustments?)

Maarten


2011/3/8 Vincent Becker vincentx.bec...@intel.com:
 This patch enables logging of text debug messages (pa_log feature) into a 
 file or a device driver.
 Example : pulseaudio --log-target=file:./mylog.txt

 Signed-off-by: Vincent Becker vincentx.bec...@intel.com
 ---
  src/daemon/cmdline.c     |    5 +++--
  src/daemon/daemon-conf.c |   34 --
  src/pulsecore/log.c      |   24 
  src/pulsecore/log.h      |    5 +
  4 files changed, 64 insertions(+), 4 deletions(-)

 diff --git a/src/daemon/cmdline.c b/src/daemon/cmdline.c
 index f6cdcdc..2c3eb67 100644
 --- a/src/daemon/cmdline.c
 +++ b/src/daemon/cmdline.c
 @@ -145,7 +145,8 @@ void pa_cmdline_help(const char *argv0) {
                                                    this time passed\n
                  --log-level[=LEVEL]               Increase or set verbosity 
 level\n
              -v                                    Increase the verbosity 
 level\n
 -                 --log-target={auto,syslog,stderr} Specify the log target\n
 +                 --log-target={auto,syslog,stderr,\n
 +                               file:PATH}          Specify the log target\n
                  --log-meta[=BOOL]                 Include code location in 
 log messages\n
                  --log-time[=BOOL]                 Include timestamps in log 
 messages\n
                  --log-backtrace=FRAMES            Include a backtrace in 
 log messages\n
 @@ -318,7 +319,7 @@ int pa_cmdline_parse(pa_daemon_conf *conf, int argc, char 
 *const argv [], int *d

             case ARG_LOG_TARGET:
                 if (pa_daemon_conf_set_log_target(conf, optarg)  0) {
 -                    pa_log(_(Invalid log target: use either 'syslog', 
 'stderr' or 'auto'.));
 +                    pa_log(_(Invalid log target: use either 'syslog', 
 'stderr', 'auto' or a valid file name 'file:path'.));
                     goto fail;
                 }
                 break;
 diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
 index e38e67a..5696f00 100644
 --- a/src/daemon/daemon-conf.c
 +++ b/src/daemon/daemon-conf.c
 @@ -28,6 +28,8 @@
  #include stdio.h
  #include string.h
  #include unistd.h
 +#include sys/stat.h
 +#include fcntl.h

  #ifdef HAVE_SCHED_H
  #include sched.h
 @@ -141,6 +143,9 @@ static const pa_daemon_conf default_conf = {
  #endif
  };

 +static int log_fd = -1;
 +
 +
  pa_daemon_conf* pa_daemon_conf_new(void) {
     pa_daemon_conf *c;

 @@ -166,6 +171,9 @@ pa_daemon_conf* pa_daemon_conf_new(void) {

  void pa_daemon_conf_free(pa_daemon_conf *c) {
     pa_assert(c);
 +    if (log_fd = 0)
 +        pa_close(log_fd);
 +
     pa_xfree(c-script_commands);
     pa_xfree(c-dl_search_path);
     pa_xfree(c-default_script_file);
 @@ -185,8 +193,30 @@ int pa_daemon_conf_set_log_target(pa_daemon_conf *c, 
 const char *string) {
     } else if (!strcmp(string, stderr)) {
         c-auto_log_target = 0;
         c-log_target = PA_LOG_STDERR;
 -    } else
 -        return -1;
 +    } else if (pa_startswith(string, file:)) {
 +        char *file_path;
 +        const char *state = NULL;
 +        unsigned i;
 +
 +        /* After second pa_split call, file_path will contain the right part 
 of file:path */
 +        for (i = 0; i  2; i++)
 +            file_path = pa_split(string, :, state);
 +
 +        /* Check if the file is regular */
 +        if ((log_fd = open(file_path, O_RDWR|O_TRUNC|O_CREAT, 0777)) = 0) {
 +            c-auto_log_target = 0;
 +            c-log_target = PA_LOG_FD;
 +            pa_log_set_fd(log_fd);
 +        }
 +        else {
 +            printf(Failed to open target file %s, error : %s\n, file_path, 
 pa_cstrerror(errno));
 +            pa_xfree(file_path);
 +            return -1;
 +        }
 +        pa_xfree(file_path);
 +    }
 +    else
 +      return -1;

     return 0;
  }
 diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
 index 2c0e267..a5e26c8 100644
 --- a/src/pulsecore/log.c
 +++ b/src/pulsecore/log.c
 @@ -71,6 +71,8 @@ static unsigned show_backtrace = 0, show_backtrace_override 
 = 0, skip_backtrace
  static pa_log_flags_t flags = 0, flags_override = 0;
  static pa_bool_t no_rate_limit = FALSE;

 +static int fdlog = -1;
 +
  #ifdef HAVE_SYSLOG_H
  static const int level_to_syslog[] = {
     [PA_LOG_ERROR] = LOG_ERR,
 @@ -128,6 +130,12 @@ void pa_log_set_flags(pa_log_flags_t _flags, 
 pa_log_merge_t merge) {
         flags = _flags;
  }

 +void pa_log_set_fd(int fd) {
 +    pa_assert(fd = 0);
 +
 +    fdlog = fd;
 +}
 +
  void pa_log_set_show_backtrace(unsigned nlevels) {
     show_backtrace = nlevels;
  }
 @@ -399,6 +407,22 @@ void pa_log_levelv_meta(
             }
  #endif

 +            case PA_LOG_FD: {
 +                if (fdlog != -1) {

Re: [pulseaudio-discuss] [PATCH 1/2] Log feature:Add a new log target to a file descriptor

2011-03-08 Thread Arun Raghavan 
Hi,
I haven't  tested your patch, but some comments on the code below.

On Tue, 2011-03-08 at 17:15 +0100, Vincent Becker wrote:
 This patch enables logging of text debug messages (pa_log feature) into a 
 file or a device driver.
 Example : pulseaudio --log-target=file:./mylog.txt
 
 Signed-off-by: Vincent Becker vincentx.bec...@intel.com
 ---
  src/daemon/cmdline.c |5 +++--
  src/daemon/daemon-conf.c |   34 --
  src/pulsecore/log.c  |   24 
  src/pulsecore/log.h  |5 +
  4 files changed, 64 insertions(+), 4 deletions(-)
 
 diff --git a/src/daemon/cmdline.c b/src/daemon/cmdline.c
 index f6cdcdc..2c3eb67 100644
 --- a/src/daemon/cmdline.c
 +++ b/src/daemon/cmdline.c
 @@ -145,7 +145,8 @@ void pa_cmdline_help(const char *argv0) {
 this time passed\n
   --log-level[=LEVEL]   Increase or set 
 verbosity level\n
   -vIncrease the verbosity 
 level\n
 - --log-target={auto,syslog,stderr} Specify the log target\n
 + --log-target={auto,syslog,stderr,\n
 +   file:PATH}  Specify the log target\n
   --log-meta[=BOOL] Include code location in 
 log messages\n
   --log-time[=BOOL] Include timestamps in 
 log messages\n
   --log-backtrace=FRAMESInclude a backtrace in 
 log messages\n
 @@ -318,7 +319,7 @@ int pa_cmdline_parse(pa_daemon_conf *conf, int argc, char 
 *const argv [], int *d
  
  case ARG_LOG_TARGET:
  if (pa_daemon_conf_set_log_target(conf, optarg)  0) {
 -pa_log(_(Invalid log target: use either 'syslog', 
 'stderr' or 'auto'.));
 +pa_log(_(Invalid log target: use either 'syslog', 
 'stderr', 'auto' or a valid file name 'file:path'.));
  goto fail;
  }
  break;
 diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
 index e38e67a..5696f00 100644
 --- a/src/daemon/daemon-conf.c
 +++ b/src/daemon/daemon-conf.c
 @@ -28,6 +28,8 @@
  #include stdio.h
  #include string.h
  #include unistd.h
 +#include sys/stat.h
 +#include fcntl.h
  
  #ifdef HAVE_SCHED_H
  #include sched.h
 @@ -141,6 +143,9 @@ static const pa_daemon_conf default_conf = {
  #endif
  };
  
 +static int log_fd = -1;
 +
 +
  pa_daemon_conf* pa_daemon_conf_new(void) {
  pa_daemon_conf *c;
  
 @@ -166,6 +171,9 @@ pa_daemon_conf* pa_daemon_conf_new(void) {
  
  void pa_daemon_conf_free(pa_daemon_conf *c) {
  pa_assert(c);
 +if (log_fd = 0)
 +pa_close(log_fd);
 +
  pa_xfree(c-script_commands);
  pa_xfree(c-dl_search_path);
  pa_xfree(c-default_script_file);
 @@ -185,8 +193,30 @@ int pa_daemon_conf_set_log_target(pa_daemon_conf *c, 
 const char *string) {
  } else if (!strcmp(string, stderr)) {
  c-auto_log_target = 0;
  c-log_target = PA_LOG_STDERR;
 -} else
 -return -1;
 +} else if (pa_startswith(string, file:)) {
 +char *file_path;
 +const char *state = NULL;
 +unsigned i;
 +
 +/* After second pa_split call, file_path will contain the right part 
 of file:path */
 +for (i = 0; i  2; i++)
 +file_path = pa_split(string, :, state);

I believe this would leak. You could just use file_path = string[5]
here.

 +/* Check if the file is regular */
 +if ((log_fd = open(file_path, O_RDWR|O_TRUNC|O_CREAT, 0777)) = 0) {

Definitely don't want to give execute permissions for that file. For
paranoia's sake, I'd use S_IRWXU.

 +c-auto_log_target = 0;
 +c-log_target = PA_LOG_FD;
 +pa_log_set_fd(log_fd);
 +}
 +else {
 +printf(Failed to open target file %s, error : %s\n, file_path, 
 pa_cstrerror(errno));
 +pa_xfree(file_path);
 +return -1;
 +}
 +pa_xfree(file_path);
 +}
 +else
 +  return -1;
  
  return 0;
  }
 diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
 index 2c0e267..a5e26c8 100644
 --- a/src/pulsecore/log.c
 +++ b/src/pulsecore/log.c
 @@ -71,6 +71,8 @@ static unsigned show_backtrace = 0, show_backtrace_override 
 = 0, skip_backtrace
  static pa_log_flags_t flags = 0, flags_override = 0;
  static pa_bool_t no_rate_limit = FALSE;
  
 +static int fdlog = -1;
 +
  #ifdef HAVE_SYSLOG_H
  static const int level_to_syslog[] = {
  [PA_LOG_ERROR] = LOG_ERR,
 @@ -128,6 +130,12 @@ void pa_log_set_flags(pa_log_flags_t _flags, 
 pa_log_merge_t merge) {
  flags = _flags;
  }
  
 +void pa_log_set_fd(int fd) {
 +pa_assert(fd = 0);
 +
 +fdlog = fd;
 +}
 +
  void pa_log_set_show_backtrace(unsigned nlevels) {
  show_backtrace = nlevels;
  }
 @@ -399,6 +407,22 @@ void pa_log_levelv_meta(
  }

Re: [pulseaudio-discuss] pulseaudio xbmc passthrough success

2011-03-08 Thread Kelly Anderson 

On 03/06/2011 02:54 PM, pl bossart wrote:

All you need is a 3D TV now...
Seriously, this is good. Can you send a link to this ported AC3Filter
code? I thought it was too Windows/DirectX-oriented to be used. I
didn't look too much since it's GPL, and LGPL is preferred in terms of
integration with proprietary components.
Thanks,
-Pierre


Pierre,

OK, I've cleaned the code up and it works really well now (for my 
purposes).  It's really efficient with processing DTS files, I haven't 
profiled Ac3 yet.  I've rolled it into my copy of xbmc and all seems to 
be working really well.  I'll be putting patches for xbmc up as well as 
soon as I've prep'd them.


You could do me a favor if you would complete sink_pulse.{cpp,h}.  I've 
been working on other things and that's really on the back burner for 
me.  Once the SinkPulse is done, we'll be able to play raw DTS/AC3 files 
pretty much the same way paplay does for other formats.  At some point 
we might even want to roll AudioFilter into paplay.


I renamed Ac3Filter to AudioFilter.  It really does a bunch more than 
just Ac3 and I've renamed interfaces, etc.


I'm still renaming files and moving the around a bit, but the headers 
should be pretty stable.


http://silka.with-linux.com/audiofilter/

I'll probably put the code in a git repository once I've got the file 
renaming out of the way.


___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss

Re: [pulseaudio-discuss] pulseaudio xbmc passthrough success

2011-03-08 Thread Kelly Anderson 

On 03/08/2011 03:32 PM, Kelly Anderson wrote:

On 03/06/2011 02:54 PM, pl bossart wrote:

All you need is a 3D TV now...
Seriously, this is good. Can you send a link to this ported AC3Filter
code? I thought it was too Windows/DirectX-oriented to be used. I
didn't look too much since it's GPL, and LGPL is preferred in terms of
integration with proprietary components.
Thanks,
-Pierre


Pierre,

OK, I've cleaned the code up and it works really well now (for my 
purposes).  It's really efficient with processing DTS files, I haven't 
profiled Ac3 yet.  I've rolled it into my copy of xbmc and all seems 
to be working really well.  I'll be putting patches for xbmc up as 
well as soon as I've prep'd them.


You could do me a favor if you would complete sink_pulse.{cpp,h}.  
I've been working on other things and that's really on the back burner 
for me.  Once the SinkPulse is done, we'll be able to play raw DTS/AC3 
files pretty much the same way paplay does for other formats.  At some 
point we might even want to roll AudioFilter into paplay.


I renamed Ac3Filter to AudioFilter.  It really does a bunch more than 
just Ac3 and I've renamed interfaces, etc.


I'm still renaming files and moving the around a bit, but the headers 
should be pretty stable.


http://silka.with-linux.com/audiofilter/

I'll probably put the code in a git repository once I've got the file 
renaming out of the way.


OK,  I've just put up the patches necessary to get passthrough working 
with Nvidia hdmi on XBMC.


___
pulseaudio-discuss mailing list
pulseaudio-discuss@mail.0pointer.de
https://tango.0pointer.de/mailman/listinfo/pulseaudio-discuss