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

Remove Baresip and libre, revert changes in H264Decoder and NvTransform specific to NVR and fix a memory leak in nvcodec decoder #389

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

kushaljain-apra
Copy link
Collaborator

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Fixes #[Issue]

Description

Summary

  1. Remove Baresip and Libre dependencies

    • Completely removed Baresip and Libre from the project.
  2. Revert changes in H264Decoder and NvTransform (specific to NVR)

    • Reverted all modifications made for NVR in the H264Decoder and NvTransform components.
    • Restored the previous, non-NVR-specific implementations.
  3. Fix memory leak in nvcodec decoder

    • Identified and resolved a memory leak in the nvcodec decoder.
    • Cuda Context was not being released when destructor is called.

Alternative(s) considered

Have you considered any alternatives? And if so, why have you chosen the approach in this PR?

Type

Type Choose one: (Bug fix)

Screenshots (if applicable)

Checklist

  • I have read the Contribution Guidelines
  • I have written Unit Tests
  • I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach

Copy link

github-actions bot commented Oct 1, 2024

Test Results Win-nocuda

  1 files  ±0    1 suites  ±0   10m 20s ⏱️ -9s
307 tests ±0  231 ✅ ±0  76 💤 ±0  0 ❌ ±0 
231 runs  ±0  155 ✅ ±0  76 💤 ±0  0 ❌ ±0 

Results for commit af79ad0. ± Comparison against base commit 9ecea0d.

Copy link
Collaborator

@yashrajsapra yashrajsapra left a comment

Choose a reason for hiding this comment

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

Look's good to me

@@ -519,8 +515,6 @@ target_include_directories ( aprapipes PRIVATE
${OpenCV_INCLUDE_DIRS}
${Boost_INCLUDE_DIRS}
${LIBMP4_INC_DIR}
${BARESIP_INC_DIR}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets keep an option open to build with baresip if required

//mShouldTriggerSOS = true;
auto frame = frame_sp(new EmptyFrame());
mDetail->compute(frame->data(), frame->size(), frame->timestamp);
LOG_ERROR << "processes sos " ;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is thie log error.

@@ -163,6 +162,7 @@ void PipeLine::run_all_threaded()
{
Module& m = *(modules[0]->controlModule);
m.myThread = boost::thread(ref(m));
Utils::setModuleThreadName(m.myThread, m.getId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

a merge with md/controlmodulepipelineFixes is required

Copy link
Collaborator

Choose a reason for hiding this comment

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

not reviewing this file

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.

4 participants