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

Adding test to resolve Issue 201 #844

Merged
merged 6 commits into from
Nov 17, 2023
Merged

Conversation

Vimal-Krishna
Copy link
Contributor

@Vimal-Krishna Vimal-Krishna commented Oct 2, 2023

Adding a new test - patest_enumerate_default_latency

Issue - #201

Outputs in this format to the cmd line:

| X |  42 |  == 1 - LG HDR 4K (AMD High Definition Audio Device) [Loopback] == | Windows WASAPI |   0.0100 |   0.0030 |   0.0000 |   0.0000 |

|   |  68 |  == Headset (@System32\drivers\bthhfenum.sys,#2;%1 Hands-Free AG Audio%0;(HC 2000BNC)) == | Windows WDM-KS |   0.0853 |   0.0100 |   0.0853 |   0.0100 |

Included the '==' markdown to disable textile formatting within the name field so that special characters like @ do not cause a formatting issue.

image

Tested the output on https://textile-lang.com/

image

Adding a new test - patest_enumerate_default_latency
Copy link
Collaborator

@RossBencina RossBencina left a comment

Choose a reason for hiding this comment

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

Thankyou! I've added some feedback for your consideration.

test/patest_enumerate_default_latency.c Outdated Show resolved Hide resolved
test/patest_enumerate_default_latency.c Show resolved Hide resolved
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.

Thanks for adding this new test program. Looks great!
Just some minor suggestions.

test/patest_enumerate_default_latency.c Outdated Show resolved Hide resolved
@RossBencina
Copy link
Collaborator

Please fix the compilation errors in CI too.

@Vimal-Krishna
Copy link
Contributor Author

Thanks for the feedback folks! I think the CI issues should be fixed as well. Let me know if the headers are ok.
(snipped to show entries with 'X')

image

@RossBencina
Copy link
Collaborator

Thanks this is looking good. The only thing I'm thinking is that it's a bit hard to interpret the 0.0000 entries because we can't see whether it's an input-only or output-only device where 0 is correct, or whether it's a bad value.

What do you think about adding another column Input/Output Channels with the channel counts for example 2/0 for a stereo input-only device.

@Vimal-Krishna
Copy link
Contributor Author

Thanks this is looking good. The only thing I'm thinking is that it's a bit hard to interpret the 0.0000 entries because we can't see whether it's an input-only or output-only device where 0 is correct, or whether it's a bad value.

What do you think about adding another column Input/Output Channels with the channel counts for example 2/0 for a stereo input-only device.

Yes, seems like a useful addition. I can make the change this weekend. How about we put in a value like N/A instead of 0.0000 for the output latency of an input-only device? And also exclude it when deciding if a device needs the 'X' marker. So the 'X' s will still be the only thing to look for when looking at the report.

@Vimal-Krishna
Copy link
Contributor Author

Devices 12 to 23 had an X marker before. Now they don't since the channel count is zero for either input or output.
textile-lang com_

Copy link
Collaborator

@RossBencina RossBencina left a comment

Choose a reason for hiding this comment

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

LGTM just going to merge these tweaks myself.

test/patest_enumerate_default_latency.c Outdated Show resolved Hide resolved
test/patest_enumerate_default_latency.c Outdated Show resolved Hide resolved
test/patest_enumerate_default_latency.c Outdated Show resolved Hide resolved
@RossBencina RossBencina merged commit 49d17ba into PortAudio:master Nov 17, 2023
11 checks passed
@philburk
Copy link
Collaborator

@Vimal-Krishna - Thanks for this contribution. The code looked very nice. We appreciate you closing this old issue from 2011.

@Vimal-Krishna
Copy link
Contributor Author

@RossBencina @philburk
Thank you! I enjoyed working on it. I hope to continue working on the project. 🙂

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