Skip to content
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

[pull] master from MusicPlayerDaemon:master #26

Open
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

pull[bot]
Copy link

@pull pull bot commented Jan 30, 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

Replace the usage of several C functions for string parsing with C++ equivalents, improving type safety and code clarity.

New Features:

  • Add support for DSD rates in the format string, allowing for more flexible audio format specifications.

Enhancements:

  • Improve type safety and code clarity by using C++ string parsing functions.

libFLAC tries to keep on seeking a stream even after a (fatal) read
error has occurred.  Let the DecoderClient decide whether proceeding
is possible.

This fixes a crash bug when playing a file over NFS and the NFS
connection fails.
The resume/seek was received asynchronously and meanwhile an error
might have occurred that needs to be handled.

This fixes another NFS-related crash bug.
Once a NFS request fails with NFS4ERR_EXPIRED, the whole connection is
broken and we need to open a new one.  I wish libnfs would report this
to us as a connection-level error, but instead only the one request
fails; therefore this workaround is an ugly kludge.

Closes #2175
@pull pull bot added the ⤵️ pull label Jan 30, 2025
Copy link

sourcery-ai bot commented Jan 30, 2025

Reviewer's Guide by Sourcery

This pull request refactors the codebase to use std::string_view instead of const char* for string handling, and introduces utility functions for parsing numbers and splitting strings. This improves code readability and maintainability, and reduces the risk of buffer overflows.

Sequence diagram for updated audio format parsing

sequenceDiagram
    participant Client
    participant AudioParser
    participant StringUtils

    Client->>AudioParser: ParseAudioFormat(string_view, mask)
    AudioParser->>StringUtils: Split(src, ':')
    StringUtils-->>AudioParser: {sample_rate, rest}
    AudioParser->>AudioParser: ParseSampleRate(sample_rate, mask)
    AudioParser->>StringUtils: Split(rest, ':')
    StringUtils-->>AudioParser: {format, channels}
    AudioParser->>AudioParser: ParseSampleFormat(format, mask)
    AudioParser->>AudioParser: ParseChannelCount(channels, mask)
    AudioParser-->>Client: AudioFormat
Loading

Class diagram showing updated string handling in InputPlugin

classDiagram
    class InputPlugin {
        +name: const char*
        +init(EventLoop&, ConfigBlock&): void
        +finish(): void
        +open(string_view uri, Mutex& mutex): InputStreamPtr
        +scan_tags(string_view uri, RemoteTagHandler& handler): RemoteTagScanner*
        +SupportsUri(string_view uri): bool
    }

    class InputStream {
        +uri: string
        +mutex: Mutex&
        +static Open(string_view uri, Mutex& mutex): InputStreamPtr
        +static OpenReady(string_view uri, Mutex& mutex): InputStreamPtr
    }

    InputPlugin ..> InputStream: creates
Loading

Class diagram showing AudioParser changes

classDiagram
    class AudioParser {
        +ParseAudioFormat(string_view src, bool mask): AudioFormat
        -ParseSampleRate(string_view src, bool mask): uint32_t
        -ParseSampleFormat(string_view src, bool mask): SampleFormat
        -ParseChannelCount(string_view src, bool mask): uint8_t
    }

    class AudioFormat {
        +sample_rate: uint32_t
        +format: SampleFormat
        +channels: uint8_t
        +Clear(): void
        +IsValid(): bool
        +IsMaskValid(): bool
    }

    AudioParser ..> AudioFormat: creates
Loading

File-Level Changes

Change Details Files
Refactor AudioParser to use std::string_view and utility functions
  • Replaced const char* with std::string_view for string arguments.
  • Introduced ParseInteger for parsing integers from string views.
  • Used StringSplit for splitting strings by a delimiter.
  • Removed manual string parsing logic and replaced it with utility functions.
  • Added string_view literals.
src/pcm/AudioParser.cxx
src/pcm/AudioParser.hxx
Update AlsaInputPlugin to use std::string_view and utility functions
  • Replaced const char* with std::string_view for string arguments.
  • Used StringAfterPrefixIgnoreCase for case-insensitive prefix matching.
  • Used StringSplit for splitting strings by a delimiter.
  • Removed manual string parsing logic and replaced it with utility functions.
src/input/plugins/AlsaInputPlugin.cxx
Update CdioParanoiaInputPlugin to use std::string_view and utility functions
  • Replaced const char* with std::string_view for string arguments.
  • Used StringAfterPrefixIgnoreCase for case-insensitive prefix matching.
  • Used ParseInteger for parsing integers from string views.
  • Used StringSplit for splitting strings by a delimiter.
  • Removed manual string parsing logic and replaced it with utility functions.
src/input/plugins/CdioParanoiaInputPlugin.cxx
Update QobuzInputPlugin to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
  • Used SkipPrefix for prefix matching.
  • Added string_view literals.
src/input/plugins/QobuzInputPlugin.cxx
src/input/plugins/QobuzTagScanner.cxx
src/input/plugins/QobuzTagScanner.hxx
Update NfsConnection to check for fatal errors
  • Added IsFatalNfsConnectionError to check for fatal NFS errors.
  • Added logic to broadcast an error when a fatal error is detected.
src/lib/nfs/Connection.cxx
Update PathTraits to include IsAbsolute
  • Added IsAbsolute to check if a path is absolute.
src/fs/Traits.hxx
Update AsyncInputStream to handle postponed exceptions
  • Added logic to check for postponed exceptions before resuming.
src/input/AsyncInputStream.cxx
Update ProxyInputStream to use template for uri
  • Updated ProxyInputStream constructor to use a template for the uri argument.
src/input/ProxyInputStream.hxx
Update InputStream to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
  • Used StringStartsWithIgnoreCase for case-insensitive prefix matching.
src/input/InputStream.cxx
src/input/InputStream.hxx
Update CurlInputPlugin to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
  • Used StringStartsWithIgnoreCase for case-insensitive prefix matching.
  • Added string_view literals.
src/input/plugins/CurlInputPlugin.cxx
Update SmbclientInputPlugin to use std::string
  • Replaced const char* with std::string for uri argument.
src/input/plugins/SmbclientInputPlugin.cxx
Update ScanTags to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/ScanTags.hxx
src/input/ScanTags.cxx
Update FingerprintCommands to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/command/FingerprintCommands.cxx
Update InputPlugin to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/InputPlugin.hxx
src/input/InputPlugin.cxx
Update FlacInput to prevent seeking when stopped
  • Added logic to prevent seeking if the decoder is stopped.
src/decoder/plugins/FlacInput.cxx
Update WavpackDecoderPlugin to use fmt::format
  • Replaced AllocatedString with fmt::format for creating the wvc url.
src/decoder/plugins/WavpackDecoderPlugin.cxx
Update FfmpegInputPlugin to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/plugins/FfmpegInputPlugin.cxx
Update MmsInputPlugin to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/plugins/MmsInputPlugin.cxx
Update Open to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/Open.cxx
Update Registry to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/Registry.hxx
src/input/Registry.cxx
Update DecoderClient to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/decoder/Client.hxx
Update SongFilter to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/song/Filter.cxx
Update SmbclientStorage to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/storage/plugins/SmbclientStorage.cxx
Update RemoteTagCache to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/RemoteTagCache.cxx
Update FileCommands to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/command/FileCommands.cxx
Update StorageCommands to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/command/StorageCommands.cxx
Update DecoderBridge to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/decoder/Bridge.cxx
src/decoder/Bridge.hxx
Update DecoderThread to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/decoder/Thread.cxx
Update AllocatedPath to use string_view in IsAbsolute
  • Updated IsAbsolute to use string_view.
src/fs/AllocatedPath.hxx
Update ThreadInputStream to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/ThreadInputStream.cxx
src/input/ThreadInputStream.hxx
Update NfsInputPlugin to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/input/plugins/NfsInputPlugin.cxx
Update Chromaprint DecoderClient to use std::string_view
  • Replaced const char* with std::string_view for string arguments.
src/lib/chromaprint/DecoderClient.hxx
test/DumpDecoderClient.cxx
test/DumpDecoderClient.hxx
test/RunChromaprint.cxx

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!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

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

MaxKellermann and others added 19 commits January 30, 2025 19:58
This reverts commit b49cfe9.  It was
a bad idea because it broke signal handlers.  I need to find a better
way to fix io_uring intialization.
This reverts commit abc8420.  This
was a bad idea, too, because it broke daemonization.
The BlockingCall() in InitUringInputPlugin() did not work because the
EventThread was not yet started.  This was never noticed until commit
e309941 which enabled `IORING_SETUP_SINGLE_ISSUER`, and suddenly
the kernel refused to accept io_uring_submit() calls from the
io_thread because io_uring_setup() had been called from the main
thread.
…iting directly to stderr.

The output gets the standard MPD log format with domain curl and a timestamp.
Using CURLOPT_DEBUGFUNCTION that is only called when CURLOPT_VERBOSE is in effect
when MPD log level is verbose.
…nullptr

May be null for the root of the files directory.
… and permission FOREGROUND_SERVICE_MEDIA_PLAYBACK

Required by newer android sdk.
MaxKellermann and others added 5 commits February 1, 2025 18:24
Require to call it before calling Run(); remove the setter from Run().
This gives class EventThread more control over when it is initialized.
Log the io_uring initialization error at MPD startup.
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.

3 participants