Fix 32-bit overflow in AudioStreamInfo::frames_to_microseconds

src/audio_stream_info.cpp

@@ -16,45 +16,24 @@
 
 namespace sendspin {
 
-// ============================================================================
-// Static helpers
-// ============================================================================
-
-static uint32_t gcd(uint32_t a, uint32_t b) {
-    while (b != 0) {
-        uint32_t t = b;
-        b = a % b;
-        a = t;
-    }
-    return a;
-}
-
 // ============================================================================
 // Constructor / Destructor
 // ============================================================================
 
 AudioStreamInfo::AudioStreamInfo(uint8_t bits_per_sample, uint8_t channels, uint32_t sample_rate)
     : sample_rate_(sample_rate), bits_per_sample_(bits_per_sample), channels_(channels) {
-    this->ms_sample_rate_gcd_ = gcd(MS_PER_SECOND, this->sample_rate_);
     this->bytes_per_sample_ = (this->bits_per_sample_ + 7) / 8;
 }
 
 // ============================================================================
 // Public API
 // ============================================================================
 
-uint32_t AudioStreamInfo::frames_to_microseconds(uint32_t frames) const {
-    return (frames * US_PER_SECOND + (this->sample_rate_ >> 1)) / this->sample_rate_;
-}
-
-uint32_t AudioStreamInfo::frames_to_milliseconds_with_remainder(uint32_t* total_frames) const {
-    uint32_t unprocessable_frames =
-        *total_frames % (this->sample_rate_ / this->ms_sample_rate_gcd_);
-    uint32_t frames_for_ms_calculation = *total_frames - unprocessable_frames;
-
-    uint32_t playback_ms = (frames_for_ms_calculation * MS_PER_SECOND) / this->sample_rate_;
-    *total_frames = unprocessable_frames;
-    return playback_ms;
+int64_t AudioStreamInfo::frames_to_microseconds(uint32_t frames) const {
+    // The product is widened to 64-bit before the multiply so it cannot overflow for any reasonable
+    // frame count.
+    return (static_cast<uint64_t>(frames) * US_PER_SECOND + (this->sample_rate_ >> 1)) /
+           this->sample_rate_;
 }
 
 bool AudioStreamInfo::operator==(const AudioStreamInfo& rhs) const {

src/audio_stream_info.h

@@ -46,7 +46,7 @@ static constexpr uint32_t DEFAULT_SAMPLE_RATE_HZ = 16000U;
  *
  * size_t bytes_for_100ms = info.ms_to_bytes(100);
  * uint32_t frames        = info.bytes_to_frames(bytes_for_100ms);
- * uint32_t duration_us   = info.frames_to_microseconds(frames);
+ * int64_t duration_us    = info.frames_to_microseconds(frames);
  * @endcode
  */
 class AudioStreamInfo {
@@ -115,13 +115,10 @@ class AudioStreamInfo {
     /// @brief Converts a frame count to microseconds
     /// @param frames Number of audio frames.
     /// @return Duration in microseconds.
-    uint32_t frames_to_microseconds(uint32_t frames) const;
-
-    /// @brief Converts frames to milliseconds, updating frames with the remainder
-    /// @param[out] frames Pointer to the frame count; updated in place with the leftover frames
-    ///             that could not be converted to a whole millisecond.
-    /// @return Whole milliseconds represented by the converted frames.
-    uint32_t frames_to_milliseconds_with_remainder(uint32_t* frames) const;
+    /// @note The intermediate frames * US_PER_SECOND product is computed in 64-bit, so this is
+    ///       avoids overflow for any reasonable frame count. The result is returned as int64_t
+    ///       to match the playtime accumulators that consume it.
+    int64_t frames_to_microseconds(uint32_t frames) const;
 
     /// @brief Returns true if both AudioStreamInfo objects describe the same format
     /// @param rhs The other AudioStreamInfo to compare against.
@@ -140,7 +137,6 @@ class AudioStreamInfo {
     size_t bytes_per_sample_;
 
     // 32-bit fields
-    uint32_t ms_sample_rate_gcd_;
     uint32_t sample_rate_;
 
     // 8-bit fields

src/sync_task.cpp

@@ -303,11 +303,8 @@ SyncTaskState SyncTask::handle_transfer_audio(SyncContext& sync_context) {
 void SyncTask::track_sent_audio(SyncContext& sync_context, size_t bytes_sent) {
     uint32_t frames_sent = sync_context.current_stream_info.bytes_to_frames(bytes_sent);
     sync_context.buffered_frames += frames_sent;
-    uint32_t remainder = frames_sent;
-    int64_t ms = sync_context.current_stream_info.frames_to_milliseconds_with_remainder(&remainder);
     sync_context.new_audio_client_playtime +=
-        US_PER_MS * ms +
-        static_cast<int64_t>(sync_context.current_stream_info.frames_to_microseconds(remainder));
+        sync_context.current_stream_info.frames_to_microseconds(frames_sent);
 }
 
 void SyncTask::send_pending_silence(SyncContext& sync_context) {
@@ -692,14 +689,8 @@ void SyncTask::process_playback_progress(SyncContext& sync_context) {
             sync_context.buffered_frames -= frames_played;
         }
 
-        uint32_t unplayed_frames = sync_context.buffered_frames;
-        int64_t unplayed_ms =
-            sync_context.current_stream_info.frames_to_milliseconds_with_remainder(
-                &unplayed_frames);
         int64_t unplayed_us =
-            US_PER_MS * unplayed_ms +
-            static_cast<int64_t>(
-                sync_context.current_stream_info.frames_to_microseconds(unplayed_frames));
+            sync_context.current_stream_info.frames_to_microseconds(sync_context.buffered_frames);
         sync_context.new_audio_client_playtime = playback_progress.finish_timestamp + unplayed_us;
     }
 }