Skip to content

Add fragmentmetadata to the kvsappcli application, CI, fix build and runtime issues for Mac#90

Merged
sirknightj merged 5 commits intomainfrom
fragment-metadata-sample-and-ci
Apr 30, 2025
Merged

Add fragmentmetadata to the kvsappcli application, CI, fix build and runtime issues for Mac#90
sirknightj merged 5 commits intomainfrom
fragment-metadata-sample-and-ci

Conversation

@sirknightj
Copy link
Contributor

Issue #, if available:

Description of changes:

Build issues addressed

  1. Add a patch to address const compile issue in media interface

Runtime issues addressed

  1. Fix a segfault during cleanup path due to double-free caused by the wrong thread id being shutdown (video thread twice).
  2. Fix occasional segfault due to string out of bounds issue in fragment ack parser.

I encountered one additional issue where when building for Release, it won't connect due to SSL verification error. The current workaround I'm using is to build with Debug. I observed disabling the compiler optimizations works as well, though not ideal.

set(CMAKE_C_FLAGS_RELEASE "-O0")  # Disable optimization in Release mode

Testing

  • New Github actions CI (automatic)
  • Ran CLI application (manually) on Mac
  • Manually checked tags are there with GetMedia and with local mkv dump
  • Ran on AMB82-Mini and checked playback in KVS console. I'll look into adding instructions for that in the future seeing Will it work on ameba Pro 2 mini #86.
image

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

LogError("Invalid argument");
/* Propagate the res error */
}
else if ((res = getTimeInIso8601(pcXAmzDate, sizeof(pcXAmzDate))) != KVS_ERRNO_NONE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewers - this section, I wanted to add these two log lines:

LogInfo("Making describe request to: %s", pServPara->pcHost);
LogInfo("Stream name: %s", pDescPara->pcStreamName);

But, LogInfo doesn't return anything, so I couldn't put it in the condition. Therefore, I moved it to the else and moved the rest of the conditions into the else as well (indent).

{
if (puHttpStatusCode != NULL)
LogInfo("Making describe request to: %s", pServPara->pcHost);
LogInfo("Stream name: %s", pDescPara->pcStreamName);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the change. The rest of this section remains untouched.

@sirknightj sirknightj requested a review from unicornss April 25, 2025 17:41
@sirknightj sirknightj requested a review from stefankiesz April 29, 2025 22:24
@sirknightj sirknightj merged commit 02ba977 into main Apr 30, 2025
2 checks passed
@sirknightj sirknightj deleted the fragment-metadata-sample-and-ci branch April 30, 2025 00:28
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.

support mkv tags

3 participants