1. 26 Mar, 2008 26 commits
    • Eric Wong's avatar
      interface: use a saner fdmax for select(2) when closing errored interfaces · 19d4f6df
      Eric Wong authored
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7218 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      19d4f6df
    • Eric Wong's avatar
      notify: cleanups · 232c9f6c
      Eric Wong authored
      * move set_nonblock{,ing}() into utils.c since we use it
      elsewhere, too
      * add proper error checking to set_nonblocking()
      * use os_compat.h instead of individually #includ-ing system headers
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7217 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      232c9f6c
    • Max Kellermann's avatar
      send notify signal after SIGCONT · 70dbc2b0
      Max Kellermann authored
      When the decoder receives SIGCONT during waitNotify(), the kernel
      restarts the read() system call.  This lets the decoder process block
      indefinitely, while the player process waits for it to react.  This
      should probably be solved with a proper signal handler which aborts
      the read() system call, but for now, we just write to the pipe to make
      it wake up.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7216 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      70dbc2b0
    • Max Kellermann's avatar
      notify the decoder instead of polling 100hz · bf05ce16
      Max Kellermann authored
      When the decoder process is faster than the player process, all
      decodedd buffers are full at some point in time.  The decoder has to
      wait for buffers to become free (finished playing).  It used to do
      this by polling the buffer status 100 times a second.
      
      This generates a lot of unnecessary CPU wakeups.  This patch adds a
      way for the player process to notify the decoder process that it may
      continue its work.
      
      We could use pthread_cond for that, unfortunately inter-process
      mutexes/conds are not supported by some kernels (Linux), so we cannot
      use this light-weight method until mpd moves to using threads instead
      of processes.  The other method would be semaphores, which
      historically are global resources with a unique name; this historic
      API is cumbersome, and I wanted to avoid it.
      
      I came up with a quite naive solution for now: I create an anonymous
      pipe with pipe(), and the decoder process reads on that pipe.  Until
      the player process sends data on it as a signal, the decoder process
      blocks.
      
      This can be optimized in a number of ways:
      
      - if the decoder process is still working (instead of waiting for
      buffers), we could save the write() system call, since there is
      nobody waiting for the notification.
      [ew: I tried this using a counter in shared memory, didn't help]
      
      - the pipe buffer will be full at some point, when the decoder thread
      is too slow.  For this reason, the writer side of the pipe is
      non-blocking, and mpd can ignore the resulting EWOULDBLOCK.
      
      - since we have shared memory, we could check whether somebody is
      actually waiting without a context switch, and we could just not
      write the notification byte.
      [ew: tried same method/result as first point above]
      
      - if there is already a notification in the pipe, we could also not
      write another one.
      [ew: tried same method/result as first/third points above]
      
      - the decoder will only consume 64 bytes at a time.  If the pipe
      buffer is full, this will result in a lot of read() invocations.
      This does not hurt badly, but on a heavily loaded system, this might
      add a little bit more load.  The preceding optimizations however
      are able eliminate the this.
      
      - finally, we should use another method for inter process
      notifications - maybe kill() or just make mpd use threads, finally.
      
      In spite of all these possibilities to optimize this code further,
      this pipe notification trick is faster than the 100 Hz poll.  On my
      machine, it reduced the number of wakeups to less than 30%.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7215 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      bf05ce16
    • Max Kellermann's avatar
      unsigned counters · 1d97bbbd
      Max Kellermann authored
      Use unsigned variables for storing the count of items or for iteration
      variables.  Since there can never be a negative number of items, it
      makes sense to use an unsigned data type here.  This change is safe
      because the unsigned values are only used for adddressing array items.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7214 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      1d97bbbd
    • Max Kellermann's avatar
      don't repeat select() · e4779fa7
      Max Kellermann authored
      The interfaces main loop repeats the select() (non-blocking) after an
      event was handled.  I do not see any reason for that, since all events
      should be handled after the first select().  This double select() does
      nothing than consume more CPU cycles.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7213 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      e4779fa7
    • Max Kellermann's avatar
      unlimited select() timeout · f9e317cc
      Max Kellermann authored
      mpd sets a 1s select() timeout for no reason.  This makes mpd wake up
      the CPU, consume some cycles just to see there is nothing to do.  We
      can save that by specifying NULL instead of a timeout.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7212 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      f9e317cc
    • Eric Wong's avatar
      wavpack_plugin: cleanups after the last commit · d78ddd1e
      Eric Wong authored
      * malloc() => xmalloc() for error checking
      * strncpy() replaced with memcpy(),
      memcpy appears perfectly safe here and mpd
      does not ever use strncpy() (see r4491)
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7211 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      d78ddd1e
    • Laszlo Ashin's avatar
      WavPack improvements · ad0d350f
      Laszlo Ashin authored
      This patch does the following:
      -enables WVC support for streams as well,
      -improves MPD inputStream <=> WavPack stream connector,
      -fixes two compile warnings (which were caused by MPD API change).
      
      Mantis #1660 <http://musicpd.org/mantis/view.php?id=1660>
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7210 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      ad0d350f
    • Eric Wong's avatar
      networking: more assertions and cleanups to size_t/unsigned changes · 99468b85
      Eric Wong authored
      Basically, I don't trust myself nor Max to not have bugs in our
      code when switching over to unsigned types, so I've added more
      assertions which will hopefully trip and force us to fix these
      bugs before somebody can exploit them :)
      
      Some cleanups for parameter parsing using strtol
      and error reporting to the user.  Also, fix some completely
      garbled indentation in inputStream_http.c
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7209 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      99468b85
    • Eric Wong's avatar
      audio.c: unsigned int functions return unsigned ints, not size_t · 9ba72643
      Eric Wong authored
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7208 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      9ba72643
    • Eric Wong's avatar
      audiofile_plugin: fix nasty long lines introduced in previous commit · 7c560d04
      Eric Wong authored
      Terminals are 80 columns and that's a hard limit, no exceptions
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7207 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      7c560d04
    • Max Kellermann's avatar
      moved code to initOutputBuffer() · 1910df96
      Max Kellermann authored
      This patch moves code which initializes the OutputBuffer struct to
      outputBuffer.c.  Although this is generally a good idea, it prepares
      the following patch.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7206 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      1910df96
    • Max Kellermann's avatar
      use size_t · 27f12c17
      Max Kellermann authored
      When dealing with in-memory lengths, the standard type "size_t" should
      be used.  Missing one can be quite dangerous, because an attacker
      could provoke an integer under-/overflow, which may provide an attack
      vector.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7205 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      27f12c17
    • Max Kellermann's avatar
      player: more assertions · 0692f6cd
      Max Kellermann authored
      Just one more assertion.  There should be more of that!
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7204 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      0692f6cd
    • Max Kellermann's avatar
      audio: use a machine word for array sizes · c0787cc9
      Max Kellermann authored
      we do not save anything by limiting a variable to an unsigned char,
      since the compiler aligns it at machine word size anyway.  however by
      using the full machine word, we save one instruction, and we remove
      the useless artificial limitation to 255.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7203 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      c0787cc9
    • Max Kellermann's avatar
      inputPlugins/oggvorbis: removed unused variables · 5a62c5cf
      Max Kellermann authored
      The local variable eof can actually be replaced with a simple "break".
      With a negative ret, the value of chunkpos can be invalidated, I am
      not sure if this might have been a bug.
      
      [ew: no, a negative ret will correspond to ret == OV_HOLE and ret
      will be reset to zero leaving chunkpos untouched (code cleaned up
      to make this more obvious]
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7202 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      5a62c5cf
    • Max Kellermann's avatar
      fix segmentation fault in song info parser · c5e1bc39
      Max Kellermann authored
      The database parser does not check whether the song object has been
      initialized yet, which may lead to a NULL pointer dereference.  Add
      this check.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7201 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      c5e1bc39
    • Max Kellermann's avatar
      fix strtok() related segmentation fault · c9e6201d
      Max Kellermann authored
      strtok() may return NULL if the input is an empty string.  The
      playlist parser did not check for that.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7200 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      c9e6201d
    • Max Kellermann's avatar
      eliminated duplicate initialization · c5b524e3
      Max Kellermann authored
      Local variables which are never read before the first assignment don't
      need initialization.  Saves a few bytes of text.  Also don't reset
      variables which are never read until function return.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7199 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      c5b524e3
    • Max Kellermann's avatar
      parse/initialize with the correct data type · 54b544c2
      Max Kellermann authored
      When we expect an integer as result, why would we use the double
      precision floating point parser?  strtol() is a better match, although
      we should probably check for overflows...
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7198 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      54b544c2
    • Max Kellermann's avatar
      fix "unreachable code" warning · b546bcfe
      Max Kellermann authored
      There is unreachable code at several positions, e.g. after an
      #if/#end, or after an endless loop.  Remove that.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7197 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      b546bcfe
    • Max Kellermann's avatar
      explicitly downcast · 66fe5806
      Max Kellermann authored
      Tools like "sparse" check for missing downcasts, since implicit cast
      may be dangerous.  Although that does not change the compiler result,
      it may make the code more readable (IMHO), because you always see when
      there may be data cut off.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7196 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      66fe5806
    • Max Kellermann's avatar
      check the result of fopen() in all code paths · 13c17c3d
      Max Kellermann authored
      The while() loop only checks for interrupted system calls (which woudl
      never happen if the signal mask were set up properly), but nobody
      checks if the fopen() actually succeeds.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7195 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      13c17c3d
    • Max Kellermann's avatar
      zero is a valid file descriptor · 33e88ff8
      Max Kellermann authored
      Although it may not happen in mpd code, it is perfectly possible for a
      newly allocated file descriptor to be zero.  For theoretical
      correctness, allow 0.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7194 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      33e88ff8
    • Max Kellermann's avatar
      moved handlePendingSignals() check into while() condition · c6ceceae
      Max Kellermann authored
      For code unification: for me, it looks ugly to do a break in the
      command in a while() block.  This belongs into the while condition.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7193 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      c6ceceae
  2. 20 Mar, 2008 1 commit
  3. 25 Feb, 2008 1 commit
    • Eric Wong's avatar
      Update ChangeLog and TODO · 7d574703
      Eric Wong authored
      ChangeLog and TODO have been updated to reflect "addid"
      improvement.
      
      esd support has been removed from the TODO, PulseAudio
      supercedes esd and we already have a PulseAudio output.
      
      Moving NAS, SUN, OSX mixer/output off into the unknown because
      nobody seems to use them or care enough to implement them (I
      sure don't).
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7187 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      7d574703
  4. 05 Feb, 2008 3 commits
  5. 29 Jan, 2008 1 commit
  6. 27 Jan, 2008 3 commits
  7. 26 Jan, 2008 5 commits
    • Eric Wong's avatar
      Revert the queue implementation and commands · 688289b2
      Eric Wong authored
      It's too ugly and broken (both technically and usability-wise)
      to be worth supporting in any stable release.
      
      In one sentence: The queue is a very crippled version of the
      playlist that takes precedence over the normal playlist.
      
      How is it crippled?
      
      * The "queueid" command only allows the queuing of songs
      ALREADY IN THE PLAYLIST!  This promotes having the entire mpd
      database of songs in the playlist, which is a stupid practice
      to begin with.
      
      * It doesn't allow for meaningful rearranging and movement
      of songs within the queue.  To move a song, you'd need to
      dequeue and requeue it (and other songs on the list).
      Why?  The playlist already allows _all_ these features
      and shows everything a client needs to know about the ordering
      of songs in a _single_ command!
      
      * Random was a stupid idea to begin with and unfortunately
      we're stuck supporting it since we've always had it.  Users
      should learn to use "shuffle" instead and not look at their
      playlists.  Implementing queue because we have the problem of
      random is just a bandage fix and digging ourselves a new hole.
      
      This protocol addition was never in a stable release of mpd, so
      reverting it will only break things for people following trunk;
      which I'm not too worried about.  I am however worried about
      long-term support of this misfeature, so I'm removing it.
      
      Additionally, there are other points:
      
      * It's trivially DoS-able:
      
      (while true; do echo queueid $song_id; done) | nc $MPD_HOST $MPD_PORT
      
      The above commands would cause the queue to become infinitely
      expanding, taking up all available memory in the system.  The
      mpd playlist was implemented as an array with a fixed (but
      configurable) size limit for this reason.
      
      * It's not backwards-compatible.  All clients would require
      upgrades (and additional complexity) to even know what the
      next song in the playlist is.  mpd is a shared architecture,
      and we should not violate the principle of least astonishment
      here.
      
      This removes the following commands:
      queueid, dequeue, queueinfo
      
      Additionally, the status field of "playlistqueue: " is removed
      from the status command.
      
      While this DoS is trivial to fix, the design is simply too
      broken to ever support in a real release.
      
      The overloading of the "addid" command and the allowing of
      negative numbers to be used as offsets is far more flexible.
      
      This improved "addid" is completely backwards-compatible with
      all clients, and does not require clients to have UI changes or
      run additional commands to display the queue.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7155 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      688289b2
    • Eric Wong's avatar
      storedPlaylist: prevent potential DoS from stored playlist commands · 29df7036
      Eric Wong authored
      While mpd has always protected against the infinite expansion of
      the main playlist by limiting its size in memory, however the
      new storedPlaylist code has never checked for this limit.
      
      Malicious (or clumsy) users could repeatedly append songs to
      stored playlists, causing files to grow increasingly large
      on disk.  Attempting to load extremely large files into memory
      will require mpd to slurp that all into memory, and ultimately
      the file would be unusable by mpd because of the configurable
      playlist size limit.
      
      Now we limit stored playlists to the max_playlist_length
      configuration variable set by the user (default is 16384).  We
      will refuse to append to playlist files if they hit that limit;
      and also refuse to load more than the specified amount of songs
      into memory.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7154 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      29df7036
    • Eric Wong's avatar
      playlist: don't allow no-op/senseless movement of songs · 3a1b3e38
      Eric Wong authored
      This disables moving the bonkered moving of the current song to
      a (negative) offset of itself (introduced in the last commit).
      
      This also short circuits no-op moves when (from == to) and
      avoid needless increasing of the playlist version and causes
      clients to issue pointless no-op plchanges commands.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7153 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      3a1b3e38
    • Eric Wong's avatar
      playlist: allow move to specify offset from current song · e213ca4f
      Eric Wong authored
      If (and only if) there is a current song in the playlist,
      (player could be stopped), allow the move destination
      argument to be specified as a negative number.
      
      This means moving any song (besides the current one) to the -1
      position will allow it to be moved to the next song in the
      playlist.  Moving any song to position -2 will move it
      to the song after the next, and so forth.
      
      Moving a song to -playlist.length will move it to the song
      _before_ the current song on the playlist; so this will
      work for repeating playlists, too.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7152 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      e213ca4f
    • Eric Wong's avatar
      command: allow "addid" command to take an optional second argument, position · 9eee1a81
      Eric Wong authored
      This will allow "addid \"song_url\" <pos>" to atomically insert a
      song at any given playlist position.
      
      If the add succeeds, but the actual movement fails (due to
      invalid position), then the song_id will be deleted before
      the command returns back to the client, and the client
      will get an error response.
      
      git-svn-id: https://svn.musicpd.org/mpd/trunk@7151 09075e82-0dd4-0310-85a5-a0d7c8717e4f
      9eee1a81