Skip to content

[pull] master from MusicPlayerDaemon:master #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
May 20, 2025

Conversation

pull[bot]
Copy link

@pull pull bot commented May 20, 2025

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.1)

Can you help keep this open source service alive? 💖 Please sponsor : )

Summary by Sourcery

Add a reopening mechanism for audio output after flush, tighten lock and error handling, refine build scripts for static linking, and improve file download safety.

Bug Fixes:

  • Return nullptr on zero-length ID3 tags to avoid invalid allocations.
  • Guard against null from std::localtime when formatting log timestamps.
  • Include correct ALSA header (<alsa/asoundlib.h>) to fix compilation.
  • Ensure audio output reopens after a flush by tracking a should_reopen flag.
  • Set the handler on InputStream in fingerprint decoding to enable proper callbacks.

Enhancements:

  • Refactor AudioOutputControl::Open to accept a moved lock and simplify unlocking.
  • Inline and mark MPDOpusDecoder destructor noexcept for stronger exception guarantees.
  • Remove obsolete condition variable parameter from InputStream documentation.
  • Use NamedTemporaryFile and os.link for atomic file downloads in the Python build script.

Build:

  • Default to static library linking and disable libsndfile examples in Meson configuration.

MaxKellermann and others added 16 commits April 28, 2025 21:55
Starting with alsa-lib 1.2.14, there is a "#warning" in alsa/error.h:

```
In file included from ../../src/lib/alsa/Error.cxx:7:
/usr/include/alsa/error.h:30:2: error: #warning "use #include <alsa/asoundlib.h>, <alsa/error.h> should not be used directly" [-Werror=cpp]
   30 | #warning "use #include <alsa/asoundlib.h>, <alsa/error.h> should not be used directly"
      |  ^~~~~~~
```

This means we can't use minimal includes anymore and must endure
ALSA's header bloat.  Pity.
InputStream::ReadFull will assert on an empty buffer, which can happen
on an empty RIFF/AIFF chunk.

Closes #2299
We switched from plain C localtime() to fmt::localtime() in commit
093122a but fmt 11.2.0 has deprecated fmt::localtime() in favor
of std::localtime()...  sigh.
This appears to fix the libsndfile subproject build which queries the
global "default_library" option.
Pass the unique_lock as rvalue reference and unlock it instead of
using ScopeUnlock, which unlocks and re-locks the mutex; then the
caller unlocks it again.  This eliminates a tiny bit of overhead.
When playback is paused at the end of a track due to "single" mode,
the source is first flushed and the output is drained (both does not
happen when pausing manually).  This sets source_state=FLUSHED which
means playback cannot be resumed until the AudioOutputSource is
reopened.  This however does not happen because
AudioOutputControl::Open() thinks it is not necessary; this method
however did not consider the AudioOutputSource flush state.  The
solution is to add yet another flag called "should_reopen" which is
set whenever the source is flushed.

Closes #2279
… files

This call was missing after OpenLocalInputStream(), leading to all
"getfingerprint" calls with io_uring to become stuck because nobody
would notify the condition variable.

Closes #2286
release v0.24.4
@pull pull bot added the ⤵️ pull label May 20, 2025
@pull pull bot merged commit 231bbdb into CartoonFan:master May 20, 2025
1 check was pending
Copy link

sourcery-ai bot commented May 20, 2025

Reviewer's Guide

This PR introduces a reopen-after-flush mechanism and refactors AudioOutputControl’s Open API to use movable locks, improves atomic file downloads in the build script, hardens log date formatting, updates destructor signatures and build defaults, and applies several small bugfixes and cleanups.

File-Level Changes

Change Details Files
Add reopen control flag and refactor AudioOutputControl Open method
  • Introduce mutex-protected should_reopen flag
  • Change Open signature to take std::unique_lock rvalue and update calls
  • Skip reopening when should_reopen is false and replace ScopeUnlock with lock.unlock()
  • Reset should_reopen in InternalOpen and set it in InternalDrain
src/output/Control.hxx
src/output/Control.cxx
src/output/Thread.cxx
Improve atomic download in build script
  • Use NamedTemporaryFile context manager instead of manual tmp path
  • Link tmp file into final path for atomic replace
python/build/download.py
Harden log date formatting against null tm
  • Check std::localtime return and return empty on null
  • Use tm reference in FmtUnsafeSV call
src/LogBackend.cxx
Mark Opus decoder destructor noexcept and inline
  • Add noexcept specifier to ~MPDOpusDecoder
  • Define destructor as inline
src/decoder/plugins/OpusDecoderPlugin.cxx
Handle zero-length ID3 segments early
  • Return nullptr immediately when size == 0 before allocation
src/tag/Id3Load.cxx
Adjust Meson build defaults for static linking and disable extras
  • Set default_library=static globally
  • Disable libsndfile examples
meson.build
Associate handler with input stream in fingerprint command
  • Call SetHandler on new input_stream
src/command/FingerprintCommands.cxx
Update ALSA error include to unified header
  • Replace <alsa/error.h> with <alsa/asoundlib.h>
src/lib/alsa/Error.cxx
Remove unused condition variable parameter docs
  • Delete cond parameter description from InputStream interface
src/input/InputStream.hxx

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

guardrails bot commented May 20, 2025

⚠️ We detected 5 security issues in this pull request:

Hard-Coded Secrets (5)
Severity Details Docs
Medium Title: Hex High Entropy String
source_hash = bc23066d87ab3168f27cef3e97d545fa63314f5c79df5ea444d41d56f962c6af
📚
Medium Title: Hex High Entropy String
patch_hash = 645bf1c335a24608b4b34f16ed10b9237bbb0131488668fb86454202239e086c
📚
Medium Title: Hex High Entropy String
patch_hash = 2cf0d047bfa5d444d5a13b85816d05895e587f61cdf9ee49a7fc40fe360ded0d
📚
Medium Title: Hex High Entropy String
source_hash = 921fc725517a694df7df38a2a3dfede6684024b5788d9de464187c612afb5918
📚
Medium Title: Hex High Entropy String
patch_hash = e3eef046409329c5c1ca8308255caa2266710fc1b9d8695fdedd04cebe42a690
📚

More info on how to fix Hard-Coded Secrets in General.


👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants