diff --git a/NEWS b/NEWS index cc77a47d4..2ca947caa 100644 --- a/NEWS +++ b/NEWS @@ -46,6 +46,11 @@ ver 0.21.25 (not yet released) * input - file: detect premature end of file - smbclient: don't send credentials to MPD clients +* decoder + - opus: apply pre-skip and end trimming + - opus: fix memory leak +* output + - osx: improve sample rate selection * Windows/Android: - fix Boost detection after breaking change in Meson 0.54 diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index 1dc2d6441..126409098 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -75,6 +75,19 @@ class MPDOpusDecoder final : public OggDecoder { OpusDecoder *opus_decoder = nullptr; opus_int16 *output_buffer = nullptr; + /** + * The pre-skip value from the Opus header. Initialized by + * OnOggBeginning(). + */ + unsigned pre_skip; + + /** + * The number of decoded samples which shall be skipped. At + * the beginning of the file, this gets set to #pre_skip (by + * OnOggBeginning()), and may also be set while seeking. + */ + unsigned skip; + /** * If non-zero, then a previous Opus stream has been found * already with this number of channels. If opus_decoder is @@ -85,6 +98,13 @@ class MPDOpusDecoder final : public OggDecoder { size_t frame_size; + /** + * The granulepos of the next sample to be submitted to + * DecoderClient::SubmitData(). Negative if unkown. + * Initialized by OnOggBeginning(). + */ + ogg_int64_t granulepos; + public: explicit MPDOpusDecoder(DecoderReader &reader) :OggDecoder(reader) {} @@ -101,6 +121,13 @@ public: bool Seek(uint64_t where_frame); private: + void AddGranulepos(ogg_int64_t n) noexcept { + assert(n >= 0); + + if (granulepos >= 0) + granulepos += n; + } + void HandleTags(const ogg_packet &packet); void HandleAudio(const ogg_packet &packet); @@ -137,10 +164,13 @@ MPDOpusDecoder::OnOggBeginning(const ogg_packet &packet) throw std::runtime_error("BOS packet must be OpusHead"); unsigned channels; - if (!ScanOpusHeader(packet.packet, packet.bytes, channels) || + if (!ScanOpusHeader(packet.packet, packet.bytes, channels, pre_skip) || !audio_valid_channel_count(channels)) throw std::runtime_error("Malformed BOS packet"); + granulepos = 0; + skip = pre_skip; + assert(opus_decoder == nullptr); assert(IsInitialized() == (output_buffer != nullptr)); @@ -177,8 +207,12 @@ MPDOpusDecoder::OnOggBeginning(const ogg_packet &packet) client.Ready(audio_format, eos_granulepos > 0, duration); frame_size = audio_format.GetFrameSize(); - output_buffer = new opus_int16[opus_output_buffer_frames - * audio_format.channels]; + if (output_buffer == nullptr) + /* note: if we ever support changing the channel count + in chained streams, we need to reallocate this + buffer instead of keeping it */ + output_buffer = new opus_int16[opus_output_buffer_frames + * audio_format.channels]; auto cmd = client.GetCommand(); if (cmd != DecoderCommand::NONE) @@ -231,22 +265,58 @@ MPDOpusDecoder::HandleAudio(const ogg_packet &packet) packet.bytes, output_buffer, opus_output_buffer_frames, 0); - if (nframes < 0) - throw FormatRuntimeError("libopus error: %s", - opus_strerror(nframes)); - - if (nframes > 0) { - const size_t nbytes = nframes * frame_size; - auto cmd = client.SubmitData(input_stream, - output_buffer, nbytes, - 0); - if (cmd != DecoderCommand::NONE) - throw cmd; - - if (packet.granulepos > 0) - client.SubmitTimestamp(FloatDuration(packet.granulepos) - / opus_sample_rate); + if (gcc_unlikely(nframes <= 0)) { + if (nframes < 0) + throw FormatRuntimeError("libopus error: %s", + opus_strerror(nframes)); + else + return; } + + /* apply the "skip" value */ + if (skip >= (unsigned)nframes) { + skip -= nframes; + AddGranulepos(nframes); + return; + } + + const opus_int16 *data = output_buffer; + data += skip * previous_channels; + nframes -= skip; + AddGranulepos(skip); + skip = 0; + + if (packet.e_o_s && packet.granulepos > 0 && granulepos >= 0) { + /* End Trimming (RFC7845 4.4): "The page with the 'end + of stream' flag set MAY have a granule position + that indicates the page contains less audio data + than would normally be returned by decoding up + through the final packet. This is used to end the + stream somewhere other than an even frame + boundary. [...] The remaining samples are + discarded. */ + ogg_int64_t remaining = packet.granulepos - granulepos; + if (remaining <= 0) + return; + + if (remaining < nframes) + nframes = remaining; + } + + /* submit decoded samples to the DecoderClient */ + const size_t nbytes = nframes * frame_size; + auto cmd = client.SubmitData(input_stream, + data, nbytes, + 0); + if (cmd != DecoderCommand::NONE) + throw cmd; + + if (packet.granulepos > 0) { + granulepos = packet.granulepos; + client.SubmitTimestamp(FloatDuration(granulepos - pre_skip) + / opus_sample_rate); + } else + AddGranulepos(nframes); } bool @@ -258,8 +328,20 @@ MPDOpusDecoder::Seek(uint64_t where_frame) const ogg_int64_t where_granulepos(where_frame); + /* we don't know the exact granulepos after seeking, so let's + set it to -1 - it will be set after the next packet which + declares its granulepos */ + granulepos = -1; + try { SeekGranulePos(where_granulepos); + + /* since all frame numbers are offset by the file's + pre-skip value, we need to apply it here as well; + we could just seek to "where_frame+pre_skip" as + well, but I think by decoding those samples and + discard them, we're safer */ + skip = pre_skip; return true; } catch (...) { return false; @@ -302,13 +384,14 @@ mpd_opus_stream_decode(DecoderClient &client, bool ReadAndParseOpusHead(OggSyncState &sync, OggStreamState &stream, - unsigned &channels) + unsigned &channels, unsigned &pre_skip) { ogg_packet packet; return OggReadPacket(sync, stream, packet) && packet.b_o_s && IsOpusHead(packet) && - ScanOpusHeader(packet.packet, packet.bytes, channels) && + ScanOpusHeader(packet.packet, packet.bytes, channels, + pre_skip) && audio_valid_channel_count(channels); } @@ -327,11 +410,12 @@ ReadAndVisitOpusTags(OggSyncState &sync, OggStreamState &stream, void VisitOpusDuration(InputStream &is, OggSyncState &sync, OggStreamState &stream, - TagHandler &handler) + ogg_int64_t pre_skip, TagHandler &handler) { ogg_packet packet; - if (OggSeekFindEOS(sync, stream, packet, is)) { + if (OggSeekFindEOS(sync, stream, packet, is) && + packet.granulepos >= pre_skip) { const auto duration = SongTime::FromScale(packet.granulepos, opus_sample_rate); @@ -351,15 +435,15 @@ mpd_opus_scan_stream(InputStream &is, TagHandler &handler) noexcept OggStreamState os(first_page); - unsigned channels; - if (!ReadAndParseOpusHead(oy, os, channels) || + unsigned channels, pre_skip; + if (!ReadAndParseOpusHead(oy, os, channels, pre_skip) || !ReadAndVisitOpusTags(oy, os, handler)) return false; handler.OnAudioFormat(AudioFormat(opus_sample_rate, SampleFormat::S16, channels)); - VisitOpusDuration(is, oy, os, handler); + VisitOpusDuration(is, oy, os, pre_skip, handler); return true; } diff --git a/src/decoder/plugins/OpusHead.cxx b/src/decoder/plugins/OpusHead.cxx index 9d302770a..596ef186a 100644 --- a/src/decoder/plugins/OpusHead.cxx +++ b/src/decoder/plugins/OpusHead.cxx @@ -18,6 +18,7 @@ */ #include "OpusHead.hxx" +#include "util/ByteOrder.hxx" #include @@ -31,12 +32,14 @@ struct OpusHead { }; bool -ScanOpusHeader(const void *data, size_t size, unsigned &channels_r) +ScanOpusHeader(const void *data, size_t size, unsigned &channels_r, + unsigned &pre_skip_r) { const auto *h = (const OpusHead *)data; if (size < 19 || (h->version & 0xf0) != 0) return false; channels_r = h->channels; + pre_skip_r = FromLE16(h->pre_skip); return true; } diff --git a/src/decoder/plugins/OpusHead.hxx b/src/decoder/plugins/OpusHead.hxx index 717f5038c..d140cee21 100644 --- a/src/decoder/plugins/OpusHead.hxx +++ b/src/decoder/plugins/OpusHead.hxx @@ -23,6 +23,7 @@ #include bool -ScanOpusHeader(const void *data, size_t size, unsigned &channels_r); +ScanOpusHeader(const void *data, size_t size, unsigned &channels_r, + unsigned &pre_skip_r); #endif diff --git a/src/output/plugins/OSXOutputPlugin.cxx b/src/output/plugins/OSXOutputPlugin.cxx index 291b5ec89..72c07796a 100644 --- a/src/output/plugins/OSXOutputPlugin.cxx +++ b/src/output/plugins/OSXOutputPlugin.cxx @@ -310,7 +310,8 @@ osx_output_score_sample_rate(Float64 destination_rate, unsigned source_rate) double int_portion; double frac_portion = modf(source_rate / destination_rate, &int_portion); // prefer sample rates that are multiples of the source sample rate - score += (1 - frac_portion) * 1000; + if (frac_portion < 0.01 || frac_portion >= 0.99) + score += 1000; // prefer exact matches over other multiples score += (int_portion == 1.0) ? 500 : 0; if (source_rate == destination_rate)