Hi,

I recently upgraded from 0.15.8 to 0.17.2 on my FreeBSD server and
noticed a few things that were different. I use MPlayer to stream OGG
from the httpd output. First, that the times shown were absurd:

> A:1060.0 (17:39.9) of 6232573214720.0 (1731270279:10:04.8)  0.1% 0%

and second, that when I paused the stream, MPlayer would try to buffer
the audio and not play it, basically doubling the time of silence when
toggling MPD's state. Both of these are addressed by teaching MPlayer
about the audio/ogg mimetype which was changed in Oct 2010[1].
Apparently no one noticed that MPlayer didn't understand the change in
that time. I've sent a patch to MPlayer upstream to recognize the
mimetype.

The third thing is that MPlayer used to show tags when streaming and
these disappeared in the upgrade. It looks like:

> [Ogg] stream 0: audio (Vorbis), -aid 0
> Ogg file format detected.
> Clip info:
>  Title: In the Beginning…
>  Creation Date: 2011-08-09
>  Album: Harmony of a Hunter
>  Artist: Mercury Adept
> ==========================================================================
> Opening audio decoder: [ffmpeg] FFmpeg/libavcodec audio decoders
> libavcodec version 54.59.100 (external)
> AUDIO: 44100 Hz, 1 ch, s16le, 110.0 kbit/15.59% (ratio: 13750->88200)
> Selected audio codec: [ffvorbis] afm: ffmpeg (FFmpeg Vorbis)
> ==========================================================================
> AO: [pulse] 44100Hz 1ch s16le (2 bytes per sample)
> Video: no video
> Starting playback...
> A: 254.0 (04:13.9) of 0.0 (unknown)  0.1% 21%
> Demuxer info Title changed to Danger in Old Tourian
> Demuxer info Artist changed to DoctorM64
> A: 240.6 (04:00.6) of 0.0 (unknown)  0.1% 23%
> Demuxer info Title changed to Earthroot
> Demuxer info Artist changed to VikingGuitar

I bisected the repository to find out when this broke and ended up with
a commit from Feb 2010[2], specifically where replay gain was removed
from the core decoder codepath and added as a filter. Hooking up the
replay_gain filter does not put the tags back into the stream.

My testcase was:

    % mplayer http://localhost:8000/mpd.ogg -dumpstream

and then playing with ogg123 after switching a song to force a tag
update. The tags are in ASCII in the stream and ogg123 recognizes them:

> Audio Device:   PulseAudio Output
>
> Playing: mpd-0.15.8.dump.ogg
> Ogg Vorbis stream: 1 channel, 44100 Hz
> Composer: Christopher Tin & Shoji Kameda
> Title: A Rapture
> Date: 2012-02-14
> Disc: 1
> Albumartist: Stereo Alchemy
> Album: God of Love
> Artist: Stereo Alchemy
> Track: 1
> Time: 00:02.40 [02:43.49] of 02:45.89  ( 81.8 kbps)  Output Buffer  88.9%

These disappeared with the mentioned commit and have been absent (as far
as git bisect has shown until 0.17.2). I played around a bit and came up
with the attached patch which, when applied to 0.17.2, brings back the
tags. It is based on a reverse diff between the first bad commit and its
parent and fiddled to reapply onto a more modern MPD.

I'm not suggesting that the patch be applied as-is since I think it's
just bringing back some functionality which was incidentally part of it
(and I don't really know what is lost by losing the serial replaygain
logic). I don't have enough knowledge of the MPD codebase to know what
is the essential difference here, only that it brings back the tags
(with no obvious negative side effects from a user POV).

Also attached are:

  - a patch to change the stream mimetype so that an unpatched MPlayer
    can be used to test;
  - the configuration file I tested with;
  - an ogginfo dump of an OGG file to test against; and
  - the configure script I used to get a clean build with minimal
    hassle while bisecting.

I can upload the stream dumps at request (I'll try to remake them of
something legally redistributable to avoid any potential issues).

I may be missing something obvious, but I didn't see anything in the
docs as to what I might need to change in the configuration file if the
tags just need to be enabled somewhere.

Thanks for the awesome project and the clean codebase (debugging and
tracking down the issues was a long process, but never one where I was
staring at code going "WTF?" at the code itself).

Thanks,

--Ben

[1]commit 5923cfcde357ca57547884819f508bff7a949620
[2]commit 752dfb3d95482c562e5d24c6ea839c4815de9a6d
music_directory                 "/mnt/mapper/media/music/ogg"
playlist_directory              "/mnt/mapper/media/music/playlists"
db_file                         "/run/user/1000/mpd/database"
log_file                        "/run/user/1000/mpd/log"
pid_file                        "/run/user/1000/mpd/pid"
state_file                      "/run/user/1000/mpd/state"
sticker_file                    "/run/user/1000/mpd/sticker.sql"
port                            "6600"
log_level                       "verbose"
save_absolute_paths_in_playlists        "no"
metadata_to_use 
"artist,album,title,track,name,date,composer,performer,disc,albumartist"
follow_outside_symlinks         "no"
follow_inside_symlinks          "no"
audio_output {
        type            "httpd"
        name            "My HTTP Stream"
        encoder         "vorbis"
        port            "8000"
        bind_to_address "127.0.0.1"
        quality         "6.0"
        format          "44100:16:1"
        max_clients     "0"
        mixer_type      "software"
        filters         "replay_gain"
}
replaygain                      "album"
buffer_before_play              "25%"
connection_timeout              "5"
filesystem_charset              "UTF-8"
filter {
        plugin          "replay_gain"
        name            "replay_gain"
}
Processing file "01 - Mercury Adept - In the Beginning….ogg"...

New logical stream (#1, serial: 0c338aae): type vorbis
Vorbis headers parsed for stream 1, information follows...
Version: 0
Vendor: Xiph.Org libVorbis I 20120203 (Omnipresent)
Channels: 2
Rate: 44100

Nominal bitrate: 192.000000 kb/s
Upper bitrate not set
Lower bitrate not set
User comments section follows...
        COMPOSER=Hirokazu Tanaka
        title=In the Beginning…
        releasecountry=XW
        totaldiscs=2
        totaltracks=17
        musicbrainz_albumartistid=89ad4ac3-39f7-470e-963a-56509c546377
        date=2011-08-09
        discnumber=1
        tracktotal=17
        albumartistsort=Various Artists
        originaldate=2011-08-09
        language=eng
        script=Latn
        album artist=
        musicbrainz_albumid=4f9b4c0f-6081-4ba4-b0b0-bb32cc62da68
        albumartist=Various Artists
        album=Harmony of a Hunter
        musicbrainz_artistid=2fae4b64-dc62-41a3-84b2-bb3543339a58
        media=Digital Media
        band=
        disctotal=2
        compilation=1
        artist=Mercury Adept
        musicbrainz_trackid=e7c5a312-7d8d-4a05-ba3a-3cdd6248d103
        artistsort=Mercury Adept
        tracknumber=1
        REPLAYGAIN_REFERENCE_LOUDNESS=89.0 dB
        REPLAYGAIN_TRACK_GAIN=-3.29 dB
        REPLAYGAIN_TRACK_PEAK=0.93211985
        REPLAYGAIN_ALBUM_GAIN=-7.29 dB
        REPLAYGAIN_ALBUM_PEAK=0.98852289
Vorbis stream 1:
        Total data length: 4856969 bytes
        Playback length: 4m:16.333s
        Average bitrate: 151.582908 kb/s
Logical stream 1 ended
>From 6f08414a9743bbb4286f2e06cb395eb41f92ce41 Mon Sep 17 00:00:00 2001
From: Ben Boeckel <maths...@gmail.com>
Date: Tue, 4 Dec 2012 23:09:25 -0500
Subject: [PATCH 1/2] mime

---
 src/encoder/vorbis_encoder.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/encoder/vorbis_encoder.c b/src/encoder/vorbis_encoder.c
index fcf2b51..0e05602 100644
--- a/src/encoder/vorbis_encoder.c
+++ b/src/encoder/vorbis_encoder.c
@@ -396,7 +396,7 @@ vorbis_encoder_read(struct encoder *_encoder, void *_dest, 
size_t length)
 static const char *
 vorbis_encoder_get_mime_type(G_GNUC_UNUSED struct encoder *_encoder)
 {
-       return  "audio/ogg";
+       return  "application/x-ogg";
 }
 
 const struct encoder_plugin vorbis_encoder_plugin = {
-- 
1.8.0.1

>From 6f99c354cc270eeff81ab44a7f6933184ee54651 Mon Sep 17 00:00:00 2001
From: Ben Boeckel <maths...@gmail.com>
Date: Tue, 4 Dec 2012 23:09:18 -0500
Subject: [PATCH 2/2] Working base

---
 Makefile.am             |   2 +
 src/decoder_api.c       |  34 ++++----------
 src/decoder_internal.c  |   9 +---
 src/decoder_internal.h  |   9 +---
 src/decoder_thread.c    |   6 +++
 src/replay_gain_state.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/replay_gain_state.h |  50 ++++++++++++++++++++
 7 files changed, 187 insertions(+), 41 deletions(-)
 create mode 100644 src/replay_gain_state.c
 create mode 100644 src/replay_gain_state.h

diff --git a/Makefile.am b/Makefile.am
index 89b6435..e6fd7bd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -180,6 +180,7 @@ mpd_headers = \
        src/refcount.h \
        src/replay_gain_config.h \
        src/replay_gain_info.h \
+       src/replay_gain_state.h \
        src/replay_gain_ape.h \
        src/sig_handlers.h \
        src/song.h \
@@ -328,6 +329,7 @@ src_mpd_SOURCES = \
        src/queue_save.c \
        src/replay_gain_config.c \
        src/replay_gain_info.c \
+       src/replay_gain_state.c \
        src/sig_handlers.c \
        src/song.c \
        src/song_update.c \
diff --git a/src/decoder_api.c b/src/decoder_api.c
index a45d0f1..3207334 100644
--- a/src/decoder_api.c
+++ b/src/decoder_api.c
@@ -26,6 +26,7 @@
 #include "buffer.h"
 #include "pipe.h"
 #include "chunk.h"
+#include "replay_gain_state.h"
 #include "replay_gain_config.h"
 
 #include <glib.h>
@@ -447,6 +448,13 @@ decoder_data(struct decoder *decoder,
 
                memcpy(dest, data, nbytes);
 
+               /* apply replay gain or normalization */
+
+               replay_gain_state_set_mode(decoder->replay_gain,
+                                          replay_gain_mode);
+               replay_gain_state_apply(decoder->replay_gain,
+                                       dest, nbytes, &dc->out_audio_format);
+
                /* expand the music pipe chunk */
 
                full = music_chunk_expand(chunk, &dc->out_audio_format, nbytes);
@@ -524,31 +532,7 @@ decoder_replay_gain(struct decoder *decoder,
        float return_db = 0;
        assert(decoder != NULL);
 
-       if (replay_gain_info != NULL) {
-               static unsigned serial;
-               if (++serial == 0)
-                       serial = 1;
-
-               if (REPLAY_GAIN_OFF != replay_gain_mode) {
-                       return_db = 20.0 * log10f(
-                               replay_gain_tuple_scale(
-                                       
&replay_gain_info->tuples[replay_gain_get_real_mode()],
-                                       replay_gain_preamp, 
replay_gain_missing_preamp,
-                                       replay_gain_limit));
-               }
-
-               decoder->replay_gain_info = *replay_gain_info;
-               decoder->replay_gain_serial = serial;
-
-               if (decoder->chunk != NULL) {
-                       /* flush the current chunk because the new
-                          replay gain values affect the following
-                          samples */
-                       decoder_flush_chunk(decoder);
-                       g_cond_signal(decoder->dc->client_cond);
-               }
-       } else
-               decoder->replay_gain_serial = 0;
+       replay_gain_state_set_info(decoder->replay_gain, replay_gain_info);
 
        return return_db;
 }
diff --git a/src/decoder_internal.c b/src/decoder_internal.c
index bc349f2..073de78 100644
--- a/src/decoder_internal.c
+++ b/src/decoder_internal.c
@@ -61,15 +61,8 @@ decoder_get_chunk(struct decoder *decoder)
 
        do {
                decoder->chunk = music_buffer_allocate(dc->buffer);
-               if (decoder->chunk != NULL) {
-                       decoder->chunk->replay_gain_serial =
-                               decoder->replay_gain_serial;
-                       if (decoder->replay_gain_serial != 0)
-                               decoder->chunk->replay_gain_info =
-                                       decoder->replay_gain_info;
-
+               if (decoder->chunk != NULL)
                        return decoder->chunk;
-               }
 
                decoder_lock(dc);
                cmd = need_chunks(dc, true);
diff --git a/src/decoder_internal.h b/src/decoder_internal.h
index d89e68c..49fb2a0 100644
--- a/src/decoder_internal.h
+++ b/src/decoder_internal.h
@@ -22,7 +22,6 @@
 
 #include "decoder_command.h"
 #include "pcm_convert.h"
-#include "replay_gain_info.h"
 
 struct input_stream;
 
@@ -73,13 +72,7 @@ struct decoder {
        /** the chunk currently being written to */
        struct music_chunk *chunk;
 
-       struct replay_gain_info replay_gain_info;
-
-       /**
-        * A positive serial number for checking if replay gain info
-        * has changed since the last check.
-        */
-       unsigned replay_gain_serial;
+       struct replay_gain_state *replay_gain;
 };
 
 /**
diff --git a/src/decoder_thread.c b/src/decoder_thread.c
index af80ed4..00659fe 100644
--- a/src/decoder_thread.c
+++ b/src/decoder_thread.c
@@ -33,6 +33,8 @@
 #include "path.h"
 #include "uri.h"
 #include "mpd_error.h"
+#include "replay_gain_state.h"
+#include "replay_gain_config.h"
 
 #include <glib.h>
 
@@ -387,6 +389,8 @@ decoder_run_song(struct decoder_control *dc,
                .dc = dc,
                .initial_seek_pending = dc->start_ms > 0,
                .initial_seek_running = false,
+               .replay_gain = replay_gain_state_new(replay_gain_preamp,
+                                                    
replay_gain_missing_preamp),
        };
        int ret;
 
@@ -413,6 +417,8 @@ decoder_run_song(struct decoder_control *dc,
        pcm_convert_deinit(&decoder.conv_state);
 
        /* flush the last chunk */
+       if (decoder.replay_gain != NULL)
+               replay_gain_state_free(decoder.replay_gain);
 
        if (decoder.chunk != NULL)
                decoder_flush_chunk(&decoder);
diff --git a/src/replay_gain_state.c b/src/replay_gain_state.c
new file mode 100644
index 0000000..9c749d6
--- /dev/null
+++ b/src/replay_gain_state.c
@@ -0,0 +1,118 @@
+/*
+ * Copyright (C) 2003-2010 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "config.h"
+#include "replay_gain_state.h"
+#include "replay_gain_config.h"
+#include "pcm_volume.h"
+
+#include <glib.h>
+
+#include <assert.h>
+
+struct replay_gain_state {
+       float preamp, missing_preamp;
+
+       enum replay_gain_mode mode;
+
+       struct replay_gain_info info;
+
+       float scale;
+};
+
+struct replay_gain_state *
+replay_gain_state_new(float preamp, float missing_preamp)
+{
+       struct replay_gain_state *state = g_new(struct replay_gain_state, 1);
+
+       state->preamp = preamp;
+       state->scale = state->missing_preamp = missing_preamp;
+       state->mode = REPLAY_GAIN_OFF;
+       replay_gain_info_init(&state->info);
+
+       return state;
+}
+
+void
+replay_gain_state_free(struct replay_gain_state *state)
+{
+       assert(state != NULL);
+
+       g_free(state);
+}
+
+static void
+replay_gain_state_calc_scale(struct replay_gain_state *state)
+{
+       assert(state != NULL);
+
+       if (state->mode == REPLAY_GAIN_OFF)
+               return;
+
+       const struct replay_gain_tuple *tuple =
+               &state->info.tuples[state->mode];
+       if (replay_gain_tuple_defined(tuple)) {
+               g_debug("computing ReplayGain scale with gain %f, peak %f",
+                       tuple->gain, tuple->peak);
+
+               state->scale = replay_gain_tuple_scale(tuple, state->preamp, 
state->missing_preamp, replay_gain_limit);
+       } else
+               state->scale = state->missing_preamp;
+}
+
+void
+replay_gain_state_set_mode(struct replay_gain_state *state,
+                          enum replay_gain_mode mode)
+{
+       assert(state != NULL);
+
+       if (mode == state->mode)
+               return;
+
+       state->mode = mode;
+
+       replay_gain_state_calc_scale(state);
+}
+
+void
+replay_gain_state_set_info(struct replay_gain_state *state,
+                          const struct replay_gain_info *info)
+{
+       assert(state != NULL);
+
+       if (info != NULL)
+               state->info = *info;
+       else
+               replay_gain_info_init(&state->info);
+
+       replay_gain_state_calc_scale(state);
+}
+
+void
+replay_gain_state_apply(const struct replay_gain_state *state,
+                       void *buffer, size_t size,
+                       const struct audio_format *format)
+{
+       assert(state != NULL);
+
+       if (state->mode == REPLAY_GAIN_OFF)
+               return;
+
+       pcm_volume(buffer, size, format->format, 
pcm_float_to_volume(state->scale));
+}
diff --git a/src/replay_gain_state.h b/src/replay_gain_state.h
new file mode 100644
index 0000000..2c7efec
--- /dev/null
+++ b/src/replay_gain_state.h
@@ -0,0 +1,50 @@
+/*
+ * Copyright (C) 2003-2011 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program 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 General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_REPLAY_GAIN_STATE_H
+#define MPD_REPLAY_GAIN_STATE_H
+
+#include "check.h"
+#include "replay_gain_info.h"
+
+#include <stddef.h>
+
+struct replay_gain_state;
+struct audio_format;
+
+struct replay_gain_state *
+replay_gain_state_new(float preamp, float missing_preamp);
+
+void
+replay_gain_state_free(struct replay_gain_state *state);
+
+void
+replay_gain_state_set_mode(struct replay_gain_state *state,
+                          enum replay_gain_mode mode);
+
+void
+replay_gain_state_set_info(struct replay_gain_state *state,
+                          const struct replay_gain_info *info);
+
+void
+replay_gain_state_apply(const struct replay_gain_state *state,
+                       void *buffer, size_t size,
+                       const struct audio_format *format);
+
+#endif
-- 
1.8.0.1

Attachment: test-mpd.sh
Description: Bourne shell script

------------------------------------------------------------------------------
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to