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

dsound: clamp smaller buffer sizes to 62.5 ms on Vista and later (cherry-pick of #782) #1008

Merged

Conversation

RossBencina
Copy link
Collaborator

see #782 for details and discussion.

Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

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

This will not compile but it passed CI!

@@ -1733,7 +1733,12 @@ static void CalculateBufferSettings( unsigned long *hostBufferSizeFrames,
unsigned long maximumPollingPeriodFrames = (unsigned long)(sampleRate * PA_DS_MAXIMUM_POLLING_PERIOD_SECONDS);
unsigned long pollingJitterFrames = (unsigned long)(sampleRate * PA_DS_POLLING_JITTER_SECONDS);

<<<<<<< HEAD
Copy link
Collaborator

Choose a reason for hiding this comment

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

This merge conflict needs to be resolved before submitting.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry Phil, I was in a rush and did not follow my standard procedure:

  1. Save the file before commit
  2. Check the diff before commit
  3. Check the diff before submitting the PR
  4. Check the diff after submitting the PR
  5. Check the CI results after submitting the PR

It should be OK now. The force pushes (1) fix the merge conflicts, (2) fix a missing {, (3) improve the short commit message.

@@ -1768,6 +1773,7 @@ static void CalculateBufferSettings( unsigned long *hostBufferSizeFrames,
unsigned long adjustedSuggestedOutputLatencyFrames =
suggestedOutputLatencyFrames - userFramesPerBuffer;

<<<<<<< HEAD
Copy link
Collaborator

Choose a reason for hiding this comment

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

Another merge conflict.

@RossBencina RossBencina force-pushed the 782_dsound_clamp_buffer_size branch 2 times, most recently from 7e34c1f to 7e3e4fa Compare March 15, 2025 00:19
This works around a DirectSound limitation where input host buffer sizes
smaller than 31.25 ms are basically unworkable and make PortAudio hang.
The workaround is to impose a minimal buffer size of 2*31.25 ms on
input-only and full-duplex streams. This is enough for the read cursor
to advance twice around the buffer, basically resulting in de facto
double buffering.

This change was tested with `paloopback` under a wide variety of
half/full-duplex, framesPerBuffer, and suggested latency parameters.
(Note the testing was done on top of PortAudio#772 as otherwise paloopback is not
usable.)

NOTE: in the input+output case this patch also increases the output
buffer size.

Fixes PortAudio#775
@RossBencina RossBencina force-pushed the 782_dsound_clamp_buffer_size branch from 7e3e4fa to 5f94391 Compare March 15, 2025 00:26
@RossBencina RossBencina changed the title dsound: clamp buffer size at 62.5 ms (cherry-pick of #782) dsound: clamp smaller buffer sizes to 62.5 ms on Vista and later (cherry-pick of #782) Mar 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants