fix(plugins/bithuman): make Pillow optional + close AvatarRunner in aclose()#6191
fix(plugins/bithuman): make Pillow optional + close AvatarRunner in aclose()#6191sgu-bithuman wants to merge 2 commits into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚩 FormData object reuse across retries may cause issues
In _send_request_with_retry (line 589), when form_data is provided, the same aiohttp.FormData object is reused across retry iterations. After the first attempt consumes the form data (reading the BytesIO streams to EOF), subsequent retries may send empty or partial payloads because the stream positions are not reset. This is a pre-existing issue not introduced by this PR, but it affects the new _send_formdata_request code path added in line 564. Worth investigating whether aiohttp handles this internally or whether FormData needs to be reconstructed for each retry.
(Refers to lines 589-612)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed this is a real issue, and as you note it's pre-existing (the FormData/BytesIO reuse predates this PR) and lives in the cloud-mode formdata path, which is unrelated to this PR's scope (making Pillow optional + closing the AvatarRunner in aclose). To keep this PR focused and easy to review, I'd prefer to address it in a follow-up — the clean fix is to reconstruct the FormData per retry attempt (e.g. pass a builder into _send_request_with_retry rather than a pre-built FormData), since aiohttp reads the streams to EOF on the first send. Happy to fold it into this PR instead if you'd rather it ship together — let me know your preference.
|
|
…close() Two fixes for the bithuman avatar plugin: 1. Pillow is imported at module top (`from PIL import Image`) but is NOT a declared dependency, so a clean install fails to import the plugin. PIL is only used for cloud-mode `avatar_image` handling; local mode (`model_path`) never touches it. Make the import optional (`try/except ModuleNotFoundError`) and guard the `isinstance(..., Image.Image)` checks, so local-mode users don't need Pillow. 2. `AvatarSession.aclose()` never closed the `AvatarRunner`: the base aclose() only removes the room participant, leaving the runner's read/forward tasks, the AV synchronizer, and the rust audio/video sources open — a native leak that accumulates per session/reconnect. Close it in aclose().
sgu-bithuman
force-pushed
the
fix/bithuman-pil-optional-and-aclose-runner
branch
from
5647b44 to
458fdd5
Compare
Addresses Devin review on livekit#6191: with Pillow optional (Image = None), the file-path branch still called Image.open(avatar_image) -> AttributeError when a local image path is passed in cloud mode without Pillow. Raise a clear BitHumanException pointing the user to `pip install Pillow` instead.
| if self._avatar_runner is not None: | ||
| await self._avatar_runner.aclose() | ||
| self._avatar_runner = None |
There was a problem hiding this comment.
🚩 Pre-existing naming inconsistency in AvatarRunner could cause AttributeError during partial-init cleanup
The new cleanup code at lines 639-641 calls self._avatar_runner.aclose(). Inside AvatarRunner.aclose() (livekit-agents/livekit/agents/voice/avatar/_runner.py:196), the attribute self._read_audio_atask is accessed. However, in AvatarRunner.__init__() at _runner.py:69, the attribute is declared as self._read_audio_task (without the 'a' before 'task'), while start() at _runner.py:92 creates self._read_audio_atask (with the 'a'). If AvatarRunner.start() were to fail after the runner is assigned to self._avatar_runner but before line 92 executes, a subsequent aclose() call would raise AttributeError. In practice this is extremely unlikely because the lines in start() before the task creation are simple event listener registrations that virtually never fail. Still, the naming inconsistency is a latent defect worth cleaning up.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Two small fixes for the
livekit-plugins-bithumanavatar plugin, found while validating a self-hosted bitHuman essence integration.1. Pillow is an undeclared dependency
avatar.pydoesfrom PIL import Imageat module top, but Pillow is not in the package's dependencies — so a cleanpip install livekit-plugins-bithumanfails to import the plugin:PIL is used only for cloud-mode
avatar_imagehandling; local mode (model_path) never touches it. This makes the import optional (try/except ModuleNotFoundError→Image = None) and guards the threeisinstance(..., Image.Image)call sites, so local-mode users don't need Pillow. (from __future__ import annotationsalready makes theImage.Imagetype hints lazy.)Verified: with Pillow uninstalled,
from livekit.plugins.bithuman import AvatarSessionimports and a localAvatarSession(model_path=..., api_secret=...)constructs successfully.2.
AvatarSession.aclose()leaks theAvatarRunneraclose()callssuper().aclose()(which only removes the room participant) +runtime.cleanup(), but never closesself._avatar_runner. That leaves the runner's_read_audio/_forward_videotasks, theAVSynchronizer, and the rustAudioSource/VideoSourceopen — a native resource leak that accumulates per session/reconnect. Nowaclose()awaitsself._avatar_runner.aclose()first.Both changes are confined to
avatar.py. No behavior change for cloud mode (Pillow present) or for the happy-path local stream.🤖 Generated with Claude Code