Skip to content

Conversation

Copy link

Copilot AI commented Dec 24, 2025

The fix-holding-frames branch (commit e8e7632) attempted to prevent frame accumulation by checking publish_task in the client's frame processing loops. This broke tests that consume directly from output_queue, causing test_overlay_mode_injects_overlay_frame to hang indefinitely.

Changes

  • Removed publish_task checks from client.py in _process_video_frames() and _process_audio_frames()
  • Added publish_task check in protocol.py egress_loop() before enqueuing to publish_queue

Rationale

The client layer should not know about protocol-level publishing concerns. By moving the check to the protocol's egress loop:

  • Frames flow through output_queue normally (tests work)
  • Frames are skipped before entering publish_queue (memory issue solved)
  • Proper abstraction boundaries maintained
# In protocol.py egress_loop()
async for frame in output_frames:
    # Record metrics...
    
    # Skip enqueuing when no publisher configured
    if self.publish_task is None:
        continue
        
    await asyncio.to_thread(enqueue_frame, frame)

This preserves the original intent of preventing frame accumulation while maintaining correct behavior for tests and direct output_queue consumers.

Original prompt

Reference: https://github.com/livepeer/pytrickle/actions/runs/20473748781/job/58834341289

Why is the test hanging? Is it flaky?


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Dec 24, 2025
Copilot AI changed the title [WIP] Investigate why the test is hanging Move publish_task check from client to protocol egress loop Dec 24, 2025
Copilot AI requested a review from eliteprox December 24, 2025 02:11
@eliteprox eliteprox marked this pull request as ready for review December 24, 2025 02:44
@eliteprox eliteprox closed this Dec 24, 2025
@eliteprox eliteprox deleted the copilot/investigate-hanging-test-issue branch December 24, 2025 17:24
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.

2 participants