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.

Sign up for GitHub

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

Jump to bottom

Add sample checks for Windows, Mac and Linux #473

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Add sample checks for Windows, Mac and Linux #473

wants to merge 4 commits into from
+215 −47

Conversation

sirknightj
Copy link
Contributor

@sirknightj sirknightj commented Feb 4, 2025 •
edited
Loading

Issue #, if available:

  • N/A

What was changed?

  • Adding verification for the samples on Windows with the mkv dump env flag
  • Fix an issue where the codec string isn't properly null-terminated in the samples. Using strcpy should be fine in this case because the arguments (constant values) are known ahead of time and not susceptible to user or network input.

Why was it changed?

  • To verify portability of the sample applications and SDK options.

How was it changed?

  • The CI will build the samples and run them

What testing was done for the changes?

  • CI should be passing for these changes:
image image

And for Mac/Linux:
image

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sirknightj sirknightj force-pushed the windows-samples branch 3 times, most recently from 95b99fc to 5568b9a Compare February 4, 2025 07:12
@sirknightj sirknightj changed the title Add VideoOnlyRealtimeStreamingSample windows check Add samples windows checks Feb 5, 2025
@sirknightj sirknightj force-pushed the windows-samples branch 4 times, most recently from 35bc5a3 to 5dcbd34 Compare February 5, 2025 16:59
@sirknightj sirknightj marked this pull request as ready for review February 5, 2025 22:44
@sirknightj sirknightj force-pushed the windows-samples branch 7 times, most recently from c1e6ca1 to 0dd91f8 Compare February 6, 2025 05:06
@sirknightj sirknightj changed the title Add samples windows checks Add sample checks for Windows, Mac and Linux Feb 6, 2025
unicornss
unicornss reviewed Feb 7, 2025
View reviewed changes
samples/KvsAudioOnlyStreamingSample.c
@@ -132,7 +132,7 @@ INT32 main(INT32 argc, CHAR* argv[])

if (argc >= 5) {
if (!STRCMP(argv[2], AUDIO_CODEC_NAME_ALAW)) {
STRNCPY(audioCodec, AUDIO_CODEC_NAME_ALAW, STRLEN(AUDIO_CODEC_NAME_ALAW));
STRCPY(audioCodec, AUDIO_CODEC_NAME_ALAW);
Copy link

@unicornss unicornss Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

SNPRINTF(audioCodec, sizeof(audioCodec), "%s", AUDIO_CODEC_NAME_ALAW);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unicornss unicornss unicornss left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
1.5.5 ci
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sirknightj @unicornss