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

PCMDriver forward tracks being played #282

Merged
merged 9 commits into from
Nov 12, 2023
Merged

PCMDriver forward tracks being played #282

merged 9 commits into from
Nov 12, 2023

Conversation

Raffaello
Copy link
Owner

@Raffaello Raffaello commented Nov 12, 2023

  • forward is it more than the total duration it will end the track
  • get "current time" (so it can be used to know how much to skip)
  • rewind if it is more than the beginning it will just from the beginning

Summary by CodeRabbit

  • New Features
    • Introduced a 'forward' function to enhance audio stream navigation.
    • Added a 'ms_toPos' function to calculate position in sound data.
  • Bug Fixes
    • Optimized stereo sound data calculation by replacing division operation with bitwise right shift.
  • Tests
    • Added a new test case 'ms_toPos' to verify the functionality of the new function.
  • Chores
    • Updated the project version from 0.16.0 to 0.17.0.
    • Modified GitHub Actions workflow to exclude specific files and directories from triggering the workflow on pull requests.
  • Refactor
    • Removed the 'append' function from the 'PCMSound' class.

@Raffaello Raffaello self-assigned this Nov 12, 2023
Copy link

coderabbitai bot commented Nov 12, 2023

Warning

Rate Limit Exceeded

@Raffaello has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 13 minutes and 19 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per repository.
Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.
Please see our FAQ for further information.

Commits Files that changed from the base of the PR and between c4181ab and 6ae2ce9.

Walkthrough

The changes primarily focus on the introduction of a forward function across various classes in the HyperSonicDrivers namespace, allowing for the advancement of the playback position in the audio stream. Additionally, an optimization is made in the sound.cpp file, and a new test case is added for the ms_toPos function. The project version is also updated in the CMakeLists.txt file.

Changes

File Path Change Summary
.../HyperSonicDrivers/utils/sound.cpp Optimized calculation by replacing division with bitwise shift. Introduced ms_toPos function.
.../HyperSonicDrivers/audio/IAudioStream.hpp Added commented-out forward function.
.../HyperSonicDrivers/audio/streams/PCMStream.cpp Introduced forward function.
.../HyperSonicDrivers/audio/streams/PCMStream.hpp Introduced forward function.
.../HyperSonicDrivers/drivers/PCMDriver.cpp Added include directive for "sound.hpp". Introduced forward function.
.../HyperSonicDrivers/drivers/PCMDriver.hpp Introduced forward function.
.../HyperSonicDrivers/utils/sound.hpp Introduced ms_toPos function.
.../HyperSonicDrivers/test/utils/TestSound.cpp Added ms_toPos test case.
.../HyperSonicDrivers/audio/PCMSound.hpp Removed append function.
.github/workflows/sonarcloud.yml Modified GitHub Actions workflow file.
CMakeLists.txt Updated project version and description.

Poem

🎵 With a hop, skip, and a jump, we move forward in the stream, 🐇💨

No more waiting, no more pause, like a dream within a dream. 🌙✨

Celebrating this day in history, with code that's clean and lean, 🎉💻

Oh, what a joy it is to code, in the season of the evergreen! 🌲🍂


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between db7430b and 16cfd08.
Files selected for processing (1)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/utils/sound.cpp (1 hunks)

@Raffaello Raffaello changed the title small improvement PCMDriver forward/rewind playing tracks Nov 12, 2023
@Raffaello Raffaello changed the title PCMDriver forward/rewind playing tracks PCMDriver forward tracks being played Nov 12, 2023
@Raffaello Raffaello added enhancement New feature or request driver driver related labels Nov 12, 2023
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 16cfd08 and 9c89311.
Files selected for processing (8)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/IAudioStream.hpp (1 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.cpp (1 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.hpp (1 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp (2 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.hpp (1 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/utils/sound.cpp (1 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/utils/sound.hpp (1 hunks)
  • sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/utils/TestSound.cpp (1 hunks)
Files skipped from review due to trivial changes (2)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/IAudioStream.hpp
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.hpp
Additional comments: 7
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.hpp (1)
  • 21-21: The new forward function is introduced correctly. Ensure that it is implemented properly in the corresponding source file and that it is used correctly throughout the codebase.
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/utils/sound.hpp (1)
  • 26-30: The new function ms_toPos is added correctly. Ensure that the function is implemented properly in the corresponding source file and that all calls to this function throughout the codebase are updated to match the new signature.
sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/utils/TestSound.cpp (1)
  • 85-105: The new test case ms_toPos looks good. It tests the ms_toPos function with a variety of inputs and checks the output against expected values. The test case is well-structured and covers both stereo and mono sound data.
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp (2)
  • 1-3: The new include directive for "sound.hpp" is appropriate for the use of utils::ms_toPos in the forward function.

  • 102-106: The new forward function seems to be correctly implemented. It iterates over all streams and calls forward on each one. However, it's important to ensure that the forward function in the PCMStream class handles any exceptions or errors that might occur when the new position exceeds the size of the sound data.

sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.cpp (1)
  • 41-45: The forward function correctly moves the current position forward by a specified number of bytes and ensures that the new position does not exceed the size of the sound data. However, it's important to note that this function does not handle the case where bytes is greater than m_sound->dataSize. If such a situation arises, m_curPos will be set to m_sound->dataSize, effectively moving the position to the end of the sound data. If this is the intended behavior, then the function is correct. If not, you may want to add error handling for this case.
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/utils/sound.cpp (1)
  • 75-83: The new function ms_toPos seems to be correctly implemented. It converts the time in milliseconds to the corresponding position in the sound data. If the sound is stereo, it correctly doubles the position. However, ensure that the return value does not exceed the size of the sound data.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 9c89311 and 1045ea9.
Files selected for processing (1)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/PCMSound.hpp (1 hunks)
Files skipped from review due to trivial changes (1)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/PCMSound.hpp

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 1045ea9 and c2d8d6f.
Files selected for processing (2)
  • .github/workflows/sonarcloud.yml (1 hunks)
  • CMakeLists.txt (1 hunks)
Files skipped from review due to trivial changes (2)
  • .github/workflows/sonarcloud.yml
  • CMakeLists.txt

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between c2d8d6f and b0da308.
Files selected for processing (3)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.hpp (1 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp (2 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.hpp (1 hunks)
Additional comments: 3
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.hpp (1)
  • 21-21: The new forward function is added correctly. Ensure that it is implemented in the corresponding .cpp file and that it behaves as expected. Also, make sure that all calls to this function throughout the codebase are updated to match the new signature.
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp (2)
  • 1-6: The inclusion of <HyperSonicDrivers/utils/sound.hpp> is new and seems to be necessary for the ms_toPos function used in the forward method. Ensure that this header file is available and correctly included.

  • 102-106: The forward function is a new addition and it seems to be correctly implemented. It iterates over m_PCMStreams_channels and calls forward on each stream with the correct parameters. Ensure that the forward method in the PCMStream class handles any exceptions that might occur during execution.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between b0da308 and c4181ab.
Files selected for processing (2)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp (2 hunks)
  • sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.hpp (1 hunks)
Additional comments: 4
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.hpp (1)
  • 34-38: The new forward functions are a good addition for providing more control over audio playback. Ensure that these functions are implemented correctly in the corresponding .cpp file and that they handle edge cases, such as attempting to forward beyond the audio stream's duration.
sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp (3)
  • 1-3: New include directive for sound.hpp is added. Ensure that this file is available and accessible in the project structure.

  • 102-106: The forward function is added to move the playback position forward by a specified duration for all channels. Ensure that the forward function in the PCMStream class is thread-safe if PCMDriver::forward can be called from multiple threads.

  • 108-115: The forward function is added to move the playback position forward by a specified duration for a specific channel group. Ensure that the forward function in the PCMStream class is thread-safe if PCMDriver::forward can be called from multiple threads.

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Raffaello Raffaello merged commit 822e477 into master Nov 12, 2023
9 checks passed
@Raffaello Raffaello deleted the pcm_forwarding branch November 12, 2023 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
driver driver related enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant