server: fix client/protocol init, customize server config, consolidate health manager #13

eliteprox · 2025-08-16T05:13:49Z

This pull request introduces several improvements and refactorings to the PyTrickle streaming library, focusing on stream state management, reliability, and documentation clarity. The most significant change is the replacement of the StreamHealthManager with a more generic StreamState, leading to updates across the stream management logic. Additional enhancements include improved publisher error recovery, better handling of protocol shutdown, and clearer documentation of stream management approaches.

Stream State Management Refactor:

Replaced StreamHealthManager with StreamState throughout the stream manager classes, updating all references and logic to use the new state management interface. This simplifies and standardizes how stream health and lifecycle are tracked and reported. (pytrickle/manager.py, pytrickle/__init__.py, pytrickle/server.py, pytrickle/health.py removed) [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]

Publisher and Protocol Reliability Improvements:

Improved the publisher's next() method to allow recovery from transient errors and retry preconnects, enhancing robustness in the face of network or encoder issues. (pytrickle/publisher.py)
Ensured protocol queues are always initialized and added safe shutdown handling to prevent errors during protocol stop. (pytrickle/protocol.py) [1] [2]
Modified publisher session management to always create a fresh session on start, preventing stale keep-alives and improving connection reliability. (pytrickle/publisher.py)

Documentation and API Clarity:

Expanded the README.md to clarify the different stream management approaches available in PyTrickle, helping users select the right architecture for their needs.

Frame Utilities:

Added a FrameFactory class to provide helper methods for constructing video and audio frames with consistent defaults. (pytrickle/frames.py)

Server Extensibility:

Introduced a RouteConfig dataclass for configuring custom routes in the server, laying groundwork for more flexible HTTP API extensions. (pytrickle/server.py)

Let me know if you need a deeper dive into any of these changes!

add health manager to server, unify state fix control handler missing consolidate HealthManager with StreamState, extend server

pschroedl · 2025-08-18T20:03:52Z

pytrickle/__init__.py

@@ -48,7 +47,6 @@
    "AudioOutput",
    "TricklePublisher",
    "TrickleSubscriber",
-    "StreamHealthManager",


Random nit: mind fixing the tab vs spaces issue on line 42 referring to "StreamProcessor"?

Fixed in adf2494

pschroedl · 2025-08-18T20:07:29Z

pytrickle/server.py

@@ -102,8 +298,9 @@ async def _handle_start_stream(self, request: web.Request) -> web.Response:
            height = params_dict.get("height", 512)
            max_framerate = params_dict.get("max_framerate", None)  # None will use default

-            # Create protocol and client (align with current Client/Protocol API)
-            protocol = TrickleProtocol(
+            #TODO: Consider adding lifecycle_lock here


I think this is worth adding async with self._lifecycle_lock: here or however you envisioned it to prevent race conditions

Added in aaea932, tested stop/start successfully

pschroedl · 2025-08-18T20:09:28Z

pytrickle/protocol.py

+            if self.subscribe_queue is not None:
+                self.subscribe_queue.put(None)
+        except Exception:
+            pass


We should log the exceptions here and on 211

Added logging in adf2494

pschroedl · 2025-08-18T20:11:15Z

pytrickle/publisher.py

+                pass
+            finally:
+                self.session = None
+        connector = aiohttp.TCPConnector(verify_ssl=False, limit=0, keepalive_timeout=5)


Keepalive and client timeout should be configurable or at least constants applied to class params.

I've now made the timeouts configurable all the way through TrickleServer and TrickleClient. See 685e12f

When initializing StreamServer, user can provide subscriber_timeout and/or publisher_timeout to configure these. I removed the keep alive mechanism as it's not necessary for these short connections.

I also updated TrickleSubscriber to follow similar constant patterns.

On second thought - some publishers may need keep-alive packets. Considering the case of no data sent for 30+ seconds on a running stream. I'll add keep-alive pings back in for publishers

keep-alive added back 0941d21. It is only configurable by Publisher currently and defaults to a class constant

pschroedl · 2025-08-18T20:12:18Z

pytrickle/publisher.py

+                # If preconnect failed due to connection reuse issues, retry once immediately
+                if self.next_writer is None and not self._should_stop():
+                    logger.info("Preconnect returned None, retrying once immediately")
+                    await asyncio.sleep(0.05)


Also should be a constant ( RETRY_DELAY_SECONDS? )

Updated to use constants in 685e12f

pschroedl · 2025-08-18T20:12:53Z

pytrickle/publisher.py

+            try:
+                await self.session.close()
+            except Exception:
+                pass


log exception

Added logging in adf2494

pschroedl · 2025-08-18T20:13:29Z

pytrickle/publisher.py

-            connector = aiohttp.TCPConnector(verify_ssl=False)
-            timeout = aiohttp.ClientTimeout(total=30)  # Reduced timeout for faster shutdown
-            self.session = aiohttp.ClientSession(connector=connector, timeout=timeout)
+        # Always create a fresh session on start to avoid stale keep-alives across streams


can you elaborate a bit more on this?

This is related to a change to close any open connection sessions when starting the publisher. It protects against memory issues if start() is called when the publisher is already running.

The previous start() would create a new connection without checking this. It is also somewhat relevant to the discussion above on timeout and keep_alive messages #13 (comment)

pschroedl · 2025-08-18T20:18:27Z

pytrickle/state.py

+        Example:
+            set_state(PipelineState.READY)
+        """
+        if state == PipelineState.WARMING_PIPELINE:


I think this could be refactored for maintainability and readability. Consider creating a mapping of state transitions and a helper referring to the map. sending you a DM

Consolidated state transition logic and removed unused code f3f7459

eliteprox force-pushed the feat/add-server-config branch 2 times, most recently from cbf207d to bea4408 Compare August 16, 2025 20:46

feat: extend server configuration to consumer

213da70

add health manager to server, unify state fix control handler missing consolidate HealthManager with StreamState, extend server

eliteprox force-pushed the feat/add-server-config branch from bea4408 to fd6567f Compare August 16, 2025 21:04

eliteprox mentioned this pull request Aug 16, 2025

feat: queue mode with improved server/client state #10

Closed

eliteprox requested a review from ad-astra-video August 16, 2025 21:12

fix: reuse TrickleClient during stream stop/start

598f8ef

eliteprox force-pushed the feat/add-server-config branch from fd6567f to 598f8ef Compare August 16, 2025 21:43

eliteprox changed the title ~~server: customize server config, consolidate health manager~~ server: fix trickleclient/protocol init, customize server config, consolidate health manager Aug 16, 2025

eliteprox changed the title ~~server: fix trickleclient/protocol init, customize server config, consolidate health manager~~ server: fix client/protocol init, customize server config, consolidate health manager Aug 16, 2025

pschroedl self-requested a review August 18, 2025 20:01

pschroedl reviewed Aug 18, 2025

View reviewed changes

eliteprox added 7 commits August 18, 2025 16:40

add logging, fix whitespace

adf2494

add lifecycle_lock for protocol init

aaea932

remove erroneous reset_frame_cache method call

5373310

update launch config to use correct process_video_example.py filename

fb54f20

config publisher/subscriber timeouts from server config

685e12f

restore keep-alive for publisher

0941d21

clean up state logic, remove unused code

f3f7459

eliteprox requested a review from pschroedl August 18, 2025 22:16

eliteprox added 2 commits August 18, 2025 21:38

improve stream state management, add tests

a6d3ce3

remove tests for another pr

f1b72a0

eliteprox force-pushed the feat/add-server-config branch from 1191a82 to f1b72a0 Compare August 19, 2025 01:57

revert lifecycle changes to server

f42d4c2

server: fix client/protocol init, customize server config, consolidate health manager #13

Are you sure you want to change the base?

server: fix client/protocol init, customize server config, consolidate health manager #13

Conversation

eliteprox commented Aug 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eliteprox Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eliteprox Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eliteprox commented Aug 16, 2025 •

edited

Loading

eliteprox Aug 18, 2025 •

edited

Loading

eliteprox Aug 18, 2025 •

edited

Loading