[webaudio]: Fix AudioBufferSourceNode duration behavior when looping [chromium/src : main]
AI Code Reviewer (Gerrit)
AI Code Reviewer added 1 comment![]( "Open in Gerrit")
double buffer_played_time() const {Blink Style Guide: Naming - Use 'CamelCase' for all function names. Function 'buffer_played_time' should be named 'BufferPlayedTime'.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mahesh Kannan (Gerrit)
Mahesh Kannan added 1 comment![]( "Open in Gerrit")
Blink Style Guide: Naming - Use 'CamelCase' for all function names. Function 'buffer_played_time' should be named 'BufferPlayedTime'.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Done
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wilson (Gerrit)
Michael Wilson added 6 comments![]( "Open in Gerrit")
Thank you, the overall approach looks good but I have some concerns about numerical issues and robustness.
double BufferPlayedTime() const {Is this used anywhere? If not, let's remove it.
std::ceil(source_frames_left / std::abs(computed_playback_rate));If the magnitude of `computed_playback_rate` is very large we may not be able to convert to `int` successfully here. Gemini suggested doing the comparison with `double` first, only casting inside the `if` block.
buffer_played_frames_ +=Could there be numerical precision issues with accumulating in this way?
UNSAFE_TODO(memset(Ideally we shouldn't introduce new unsafe operations. Perhaps we should land some pre-work to convert `destination_channels_` into spans.
// We're looping a grain with a grain duration specified. The node will stopDid this comment get cut off?
Related details
- Mahesh Kannan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mahesh Kannan (Gerrit)
Mahesh Kannan added 3 comments![]( "Open in Gerrit")
Is this used anywhere? If not, let's remove it.
Removed, I added that incase we need to add the information for debugging `duration` issues
std::ceil(source_frames_left / std::abs(computed_playback_rate));If the magnitude of `computed_playback_rate` is very large we may not be able to convert to `int` successfully here. Gemini suggested doing the comparison with `double` first, only casting inside the `if` block.
Done
// We're looping a grain with a grain duration specified. The node will stopDid this comment get cut off?
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mahesh Kannan (Gerrit)
Mahesh Kannan added 1 comment![]( "Open in Gerrit")
Ideally we shouldn't introduce new unsafe operations. Perhaps we should land some pre-work to convert `destination_channels_` into spans.
Done
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |