Re: [Musicpd-dev-team] concerns about wakeups
Hi Max, 2009/3/23 Max Kellermann m...@duempel.org: On 2009/03/22 11:43, Kodest kod...@gmail.com wrote: Just tell me when i can check the wakeups again :) Submit a bug report, we'll add it to the 0.15 roadmap. http://musicpd.org/mantis/view.php?id=2183 Is there any docs on how the output mechanism works? I would be interested in how the audio chunks travel among the different threads at the output subsystem, but the code is quite complex. Maybe I can help if i understand this one. (Only maybe because my time is really limited nowadays.) Yes, I've been missing your regular contributions in the past months ;) Basically: the decoder thread feeds music_chunk objects into a music_pipe. The player thread takes them from there, and appends them to the output's music_pipe instance. The output threads now consume them. When all output threads have consumed a music_chunk, it is freed (returned to the music_buffer). The frequent wakeups (supposedly) come from the threads waiting for more chunks, or waiting for enough room in the pipe, or waiting for free chunks in the music_buffer. They're just using g_usleep(10ms) to synchronize, because I havn't had time to think about the proper solution yet (using GCond or struct notify). For most setups (small buffers, low latency), the music_pipe/chunk/buffer code is more efficient than the old method (fully synchronous buffer passing), but for your setup, it's worse. We will optimize MPD for all possible use cases. I tried with default sized alsa buffer (period_size=1024; buffer_size=8192). I get the same amount of wakeups caused by mpd (~950 per sec). Only the number of interrupts changed to 50 per sec from 10. Btw low latency can be achieved through large buffers as well; this could be useful reading: http://0pointer.de/blog/projects/pulse-glitch-free.html I believe it'll take some time to fully understand that code, so if you have little time, I promise to fix it this week. Thanks for the info, I will look at that code too. Regards, Kodest -- Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are powering Web 2.0 with engaging, cross-platform capabilities. Quickly and easily build your RIAs with Flex Builder, the Eclipse(TM)based development software that enables intelligent coding and step-through debugging. Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] concerns about wakeups
Hi Max, 2009/3/22 Max Kellermann m...@duempel.org: On 2009/03/21 14:20, Kodest kod...@gmail.com wrote: I took a look at powertop yesterday, and found that mpd has begun to generate many wakeups (much more than it used to do before, ~950 per sec). So, I did a bisect and I think I have found the guilty commit: 3291666b570b1d20f59db42936eaa37dbeb9ca65 As far as I see, this is a reorganization in the output system. Please look at this. You're talking about MPD while playing, aren't you? Yes, i measured wakeups while mpd was playing mp3s. (Actually mp3s are irrelevant, flac does the same; the problem is not at the input/decoder parts.) How many wakeups before that commit (while playing)? Last good commit (commit ab3d7c29dae44f39df28c85e26b108c44fdbc4bf): mpd doesn't even come up. Only the interrupts can be seen in powertop generated by the sound hw (only ~2-3 per sec, because i use large alsa buffer). I think this is quite good! First bad commit (commit 3291666b570b1d20f59db42936eaa37dbeb9ca65) 95.6% (947.0) mpd : do_nanosleep (hrtimer_wakeup) Last commit at the moment (commit 71cd24954a34bc9fb0fdf6505616ba79b8320a5a) 95.6% (948.3) mpd : do_nanosleep (hrtimer_wakeup) I know that it's a bit inefficient right now, and the code has several comments XXX synchronize in a better way. I will work on that before the 0.15 release (and before the beta test, that's important). Just tell me when i can check the wakeups again :) Is there any docs on how the output mechanism works? I would be interested in how the audio chunks travel among the different threads at the output subsystem, but the code is quite complex. Maybe I can help if i understand this one. (Only maybe because my time is really limited nowadays.) Regards, Kodest -- Apps built with the Adobe(R) Flex(R) framework and Flex Builder(TM) are powering Web 2.0 with engaging, cross-platform capabilities. Quickly and easily build your RIAs with Flex Builder, the Eclipse(TM)based development software that enables intelligent coding and step-through debugging. Download the free 60 day trial. http://p.sf.net/sfu/www-adobe-com ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] command: eliminate gcc warning
Hi Max, Argument cmd of function command_available() is not used if mpd was configured without sqlite. You can find the patch for this one in my repo. It's just a G_GNUC_UNUSED. Regards, Kodest -- This SF.net email is sponsored by: SourcForge Community SourceForge wants to tell your story. http://p.sf.net/sfu/sf-spreadtheword ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] fix add url regression
Hi, I noticed that the add url functionality has gone in the last time somehow: mpc add http://... reported that the scheme was unknown. I tracked down that ls.c didn't see HAVE_CURL because of the missing config.h. I assume ls.c included config.h implicitly earlier but the huge amount of modifications made the inclusion go away. So, an explicit include solves the problem. I fixed this in my repo: http://git.musicpd.org/cgit/kod/mpd.git/commit/?id=a12b57e1e938e92b906c22ee132ce9d4f90e0124 Regards, Kodest -- ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] general code improvement: assert_static
Hi, 2008/11/22 Max Kellermann [EMAIL PROTECTED]: On 2008/11/21 21:29, Kodest [EMAIL PROTECTED] wrote: I think we should consider using this at least where I pointed in the wavpack plug-in, but maybe there are more places in the mpd code where assert()s could be replaced by assert_static()s, or where assert_static()s could be introduced. So, assert_static() should be defined in a common header file. Tell me if we have already a solution to this problem. We don't have that currently, but it sounds like a good idea. Keep in mind that assertions don't generate any machine code with -DNDEBUG, so the advantage over real assert() is very small in this regard; it's however useful because you don't have to execute the code to see the error. But if you see places where a compile-time assertion is more useful, why not. I'll accept patches. I have two patches, both of them can be found in my repo. 457a6f4b utils: introduce assert_static() a493aafe wavpack: use assert_static() I tested it with gcc-4.1.2. Good case warnings/errors: none Bad case warnings/errors: decoder/wavpack_plugin.c: In function 'format_samples_int': decoder/wavpack_plugin.c:75: warning: division by zero decoder/wavpack_plugin.c:75: error: enumerator value for 'assert_static__' is not an integer constant This is not the nice Assertion failed ... message, but it refers to assert_static, so the situation can be figured out. Please comment. Regards, Kodest - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] aac fixes
Hi,
On 20 November 2008 14:59, Max Kellermann [EMAIL PROTECTED] wrote:
What did configure say about broken libfaad headers on your machine?
Look what config.log writes.
config.log snippet:
--
configure:9555: checking for broken libfaad headers
configure:9578: gcc -std=gnu99 -c -pthread -g -O2 -Werror
-pthread -g -O2 -Werror conftest.c 5
cc1: warnings being treated as errors
conftest.c: In function 'main':
conftest.c:10: warning: passing argument 4 of 'NeAACDecInit2' from
incompatible pointer type
configure:9584: $? = 1
configure: failed program was:
|
| #include faad.h
| #include stddef.h
| #include stdint.h
|
| int main() {
| unsigned char channels;
| uint32_t sample_rate;
|
| faacDecInit2(NULL, NULL, 0, sample_rate, channels);
| return 0;
| }
|
configure:9595: result: broken
--
As expected.
And the following line comes up in the configure stdout:
checking for broken libfaad headers... broken
HAVE_FAAD_LONG is defined. So the following lines in the aac plugin
are compiled which triggers the warnings:
259: unsigned long *sample_rate_r = (unsigned long*)sample_rate;
323: unsigned long *sample_rate_r = (unsigned long*)sample_rate;
My very simple idea to resolve this situation (not the union-based
one): cast the int32_t pointer argument to void* at the calling like
the attached patch. It compiles without any warnings and runs as well.
What do you think about it? (The patch makes the HAVE_FAAD_LONG define
unneeded.)
Regards,
Kodest
diff --git a/src/decoder/aac_plugin.c b/src/decoder/aac_plugin.c
index d23d43b..68bb9c3 100644
--- a/src/decoder/aac_plugin.c
+++ b/src/decoder/aac_plugin.c
@@ -252,14 +252,6 @@ static float getAacFloatTotalTime(const char *file)
faacDecHandle decoder;
faacDecConfigurationPtr config;
uint32_t sample_rate;
-#ifdef HAVE_FAAD_LONG
- /* neaacdec.h declares all arguments as unsigned long, but
- internally expects uint32_t pointers. To avoid gcc
- warnings, use this workaround. */
- unsigned long *sample_rate_r = (unsigned long*)sample_rate;
-#else
- uint32_t *sample_rate_r = sample_rate;
-#endif
unsigned char channels;
struct input_stream inStream;
long bread;
@@ -279,10 +271,14 @@ static float getAacFloatTotalTime(const char *file)
fillAacBuffer(b);
#ifdef HAVE_FAAD_BUFLEN_FUNCS
- bread = faacDecInit(decoder, b.buffer, b.bytesIntoBuffer,
- sample_rate_r, channels);
+ bread = faacDecInit(
+ decoder, b.buffer, b.bytesIntoBuffer,
+ (void *)sample_rate, channels
+ );
#else
- bread = faacDecInit(decoder, b.buffer, sample_rate_r,
channels);
+ bread = faacDecInit(
+ decoder, b.buffer, (void *)sample_rate, channels
+ );
#endif
if (bread = 0 sample_rate 0 channels 0)
length = 0;
@@ -316,14 +312,6 @@ aac_stream_decode(struct decoder *mpd_decoder, struct
input_stream *inStream)
faacDecConfigurationPtr config;
long bread;
uint32_t sample_rate;
-#ifdef HAVE_FAAD_LONG
- /* neaacdec.h declares all arguments as unsigned long, but
- internally expects uint32_t pointers. To avoid gcc
- warnings, use this workaround. */
- unsigned long *sample_rate_r = (unsigned long*)sample_rate;
-#else
- uint32_t *sample_rate_r = sample_rate;
-#endif
unsigned char channels;
unsigned int sampleCount;
char *sampleBuffer;
@@ -358,10 +346,14 @@ aac_stream_decode(struct decoder *mpd_decoder, struct
input_stream *inStream)
}
#ifdef HAVE_FAAD_BUFLEN_FUNCS
- bread = faacDecInit(decoder, b.buffer, b.bytesIntoBuffer,
- sample_rate_r, channels);
+ bread = faacDecInit(
+ decoder, b.buffer, b.bytesIntoBuffer,
+ (void *)sample_rate, channels
+ );
#else
- bread = faacDecInit(decoder, b.buffer, sample_rate_r, channels);
+ bread = faacDecInit(
+ decoder, b.buffer, (void *)sample_rate, channels
+ );
#endif
if (bread 0) {
ERROR(Error not a AAC stream.\n);
-
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100url=/___
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
Re: [Musicpd-dev-team] aac fixes
Hi, On 20 November 2008 17:39, Max Kellermann [EMAIL PROTECTED] wrote: What type does NeAACDecInit2() expect on your machine? It's unsigned long *samplerate on the latest unmodified release (2.6.1), and it compiles without warnings here. I think you meant NeAACDecInit(), not NeAACDecInit2(). The mpd code calls faacDecInit() which is defined to NeAACDecInit() in neaacdec.h. Btw this doesn't matter while the signatures of these two functions are the same in this header for me: -- /* Init the library based on info from the AAC file (ADTS/ADIF) */ long NEAACDECAPI NeAACDecInit(NeAACDecHandle hDecoder, unsigned char *buffer, unsigned long buffer_size, unsigned long *samplerate, unsigned char *channels); /* Init the library using a DecoderSpecificInfo */ char NEAACDECAPI NeAACDecInit2(NeAACDecHandle hDecoder, unsigned char *pBuffer, unsigned long SizeOfDecoderSpecificInfo, unsigned long *samplerate, unsigned char *channels); -- Versions: faad2: 2.6.1-r2 (-r2 means this is patched by Gentoo, however, we have the same unsigned long * for the samplerate argument.) neaacdec.h: $Id: neaacdec.h,v 1.11 2007/11/01 12:33:29 menno Exp $ gcc: gcc (GCC) 4.1.2 (Gentoo 4.1.2 p1.1) maybe differenc gcc version? [[ my embarrassing question: do you have x86_64, like me? ]] ;) Regards, Kodest - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] wavpack: redo using audio_format_frame_size
Hi, Before I did my first git rebase, a patch from Max was lost after renaming some variables. This patch brings it back: http://git.musicpd.org/cgit/kod/mpd.git/commit/?id=05f4629fa3785b9596024d24eaea6f9f5afd75fa Available in git://git.musicpd.org/kod/mpd.git Regards, Kodest - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
[Musicpd-dev-team] aac fixes
Hi, I have fixed a segfault in aac plugin and a minor gcc warning. aac: don't try to free static buffer aac: fix compiler warnings on amd64 These can be found in the master branch of my git repo: http://git.musicpd.org/cgit/kod/mpd.git/ I think it can be pulled by: git pull git://git.musicpd.org/kod/mpd.git refs/heads/master But I haven't tested this one. Now my repo is rebased on the master git repo as Max suggested. Regards, Kodest - This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100url=/ ___ Musicpd-dev-team mailing list Musicpd-dev-team@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team
