Skip to content

Conversation

@yashrajsapra
Copy link
Collaborator

@yashrajsapra yashrajsapra commented Jul 13, 2025

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

Fixes #419

Description

Refactord Pop function to block the thread, when no frames are their in the queue

Type

Type Choose one: (Bug fix | Feature | Documentation | Testing | Other)

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

@yashrajsapra yashrajsapra self-assigned this Jul 13, 2025
@github-actions
Copy link
Contributor

Test Results Linux

  1 files  ±0    1 suites  ±0   10m 17s ⏱️ ±0s
319 tests ±0  231 ✅ ±0  83 💤 ±0  5 ❌ ±0 
236 runs  ±0  148 ✅ ±0  83 💤 ±0  5 ❌ ±0 

For more details on these failures, see this check.

Results for commit baaeca2. ± Comparison against base commit d8a11b7.

@github-actions
Copy link
Contributor

Test Results Win-nocuda

  1 files  ±0    1 suites  ±0   10m 36s ⏱️ +2s
313 tests ±0  236 ✅ ±0  77 💤 ±0  0 ❌ ±0 
236 runs  ±0  159 ✅ ±0  77 💤 ±0  0 ❌ ±0 

Results for commit baaeca2. ± Comparison against base commit d8a11b7.

Copy link
Collaborator

@kumaakh kumaakh left a comment

Choose a reason for hiding this comment

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

@yashrajsapra the FrameContainerQueueAdapter was written to handle the case of dropped P frames during push. basically if a P frame is dropped we wanted to drop all the following frames till an I frame showed up.

The adaptation of pop() was unnecessarily added assuming that in future someone will need it and that created this bad outcome...

The way you have fixed it, shows that the adaptation is redundant...
pop() is now calling on_pop_success() and not adding any additonal value...
May be we can just implement it as
retrun mAdaptee->pop();

and let the Module react to the reurn value being not null ?

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.

High CPU Consumption

3 participants