-
Notifications
You must be signed in to change notification settings - Fork 0
Fix aja in not working correctly in linux #13
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
base: nodos-1.3
Are you sure you want to change the base?
Conversation
This was probably caused by aja driver resetting the device state after an application releases the device. Fixed by acquiring/releasing with refcounting provided by aja sdk itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes an issue with AJA device handling on Linux by switching from basic device acquisition/release to reference-counted acquisition/release using the AJA SDK's built-in refcounting mechanism.
Key changes:
- Replaced
AcquireStreamForApplicationwithAcquireStreamForApplicationWithReference - Replaced
ReleaseStreamForApplicationwithReleaseStreamForApplicationWithReference - Modified logging logic to avoid redundant messages when device is already acquired by the same process
- Bumped plugin version from 2.7.0 to 2.7.1
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Source/AJADevice.cpp | Updated device acquisition/release to use reference-counted API calls and adjusted logging logic |
| AJA.noscfg | Version bump to 2.7.1 to reflect the bug fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (curPid == nosPid) | ||
| return true; | ||
| auto curApp = FourCCToString(curAppFourCC); | ||
| if (curPid != 0) | ||
| nosEngine.LogW("Device %s is already acquired by application %s (PID %d). Trying to reclaim it.", GetDisplayName().c_str(), curApp.c_str(), curPid); | ||
| { | ||
| auto curApp = FourCCToString(curAppFourCC); | ||
| if (curPid != 0) | ||
| nosEngine.LogW("Device %s is already acquired by application %s (PID %d). Trying to reclaim it.", GetDisplayName().c_str(), curApp.c_str(), curPid); | ||
| } |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for handling device acquisition has been changed incorrectly. Previously, the warning was logged when the device was acquired by ANY other application (curPid != 0, and after the early return check, this meant curPid != nosPid). Now the warning is only logged when curPid == nosPid (same process), which is contradictory to the message saying "Trying to reclaim it". The warning should be logged when the device is acquired by a DIFFERENT application, not the same one. The condition should be 'if (curPid != nosPid)' instead of wrapping the warning in the 'if (curPid == nosPid)' block.
| { | ||
| auto curApp = FourCCToString(curAppFourCC); | ||
| if (curPid != 0) | ||
| nosEngine.LogW("Device %s is already acquired by application %s (PID %d). Trying to reclaim it.", GetDisplayName().c_str(), curApp.c_str(), curPid); |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: this line uses tabs while the surrounding code uses spaces for indentation. This should match the indentation style of the rest of the file.
| return false; | ||
| } | ||
| nosEngine.LogD("Device %s acquired by Nodos.", GetDisplayName().c_str()); | ||
| if(nosPid != curPid) |
Copilot
AI
Jan 8, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after 'if' keyword. The code style should have a space between 'if' and the opening parenthesis for consistency with the rest of the codebase.
| if(nosPid != curPid) | |
| if (nosPid != curPid) |
This was probably caused by aja driver resetting the device state after an application releases the device. Fixed by acquiring/releasing with refcounting provided by aja sdk itself.