Skip to content

fix(plugins/bithuman): make Pillow optional + close AvatarRunner in aclose() #6191

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.

Sign up for GitHub

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

Open
sgu-bithuman wants to merge 2 commits into
base: main
Choose a base branch
from
+19 −4
Open

fix(plugins/bithuman): make Pillow optional + close AvatarRunner in aclose() #6191

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
458fdd5
fix(plugins/bithuman): make Pillow optional + close AvatarRunner in a…
sgu-bithuman Jun 23, 2026
7347fdd
fix(plugins/bithuman): guard Image.open() when Pillow is absent
sgu-bithuman Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions livekit-plugins/livekit-plugins-bithuman/livekit/plugins/bithuman/avatar.py
View file
Open in desktop
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.

@devin-ai-integration devin-ai-integration Bot Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 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)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@sgu-bithuman sgu-bithuman Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
from __future__ import annotations

import asyncio
import base64
import io
import os
import sys
from collections.abc import AsyncGenerator, AsyncIterator
from typing import TYPE_CHECKING, Literal
from urllib.parse import parse_qs, urlparse

import aiohttp
import cv2
import numpy as np
from loguru import logger as _logger

Check failure on line 15 in livekit-plugins/livekit-plugins-bithuman/livekit/plugins/bithuman/avatar.py

View workflow job for this annotation

GitHub Actions / ruff

ruff (I001) 

livekit-plugins/livekit-plugins-bithuman/livekit/plugins/bithuman/avatar.py:1:1: I001 Import block is un-sorted or un-formatted
  help: Organize imports
from PIL import Image
try:
from PIL import Image
except ModuleNotFoundError: # Pillow only needed for cloud-mode avatar_image
Image = None

from livekit import api, rtc
from livekit.agents import (
Expand Down Expand Up @@ -196,10 +199,15 @@
raise BitHumanException("BITHUMAN_API_URL are required for cloud mode")

self._avatar_image: Image.Image | str | None = None
if isinstance(avatar_image, Image.Image):
if Image is not None and isinstance(avatar_image, Image.Image):
self._avatar_image = avatar_image
elif isinstance(avatar_image, str):
if os.path.exists(avatar_image):
if Image is None:
raise BitHumanException(
"Pillow is required to load an avatar image from a file path. "
"Install it with: pip install Pillow"
)
self._avatar_image = Image.open(avatar_image)
elif avatar_image.startswith(("http://", "https://")):
self._avatar_image = avatar_image
Expand Down Expand Up @@ -423,7 +431,7 @@
}

# Handle avatar image - convert to base64 for JSON serialization
if isinstance(self._avatar_image, Image.Image):
if Image is not None and isinstance(self._avatar_image, Image.Image):
img_byte_arr = io.BytesIO()
self._avatar_image.save(img_byte_arr, format="JPEG", quality=95)
img_byte_arr.seek(0)
Expand Down Expand Up @@ -502,7 +510,7 @@
form_data.add_field("async_mode", "true" if async_mode else "false")

# Handle avatar image - send as file upload or URL
if isinstance(self._avatar_image, Image.Image):
if Image is not None and isinstance(self._avatar_image, Image.Image):
# Convert PIL Image to bytes and upload as file
img_byte_arr = io.BytesIO()
self._avatar_image.save(img_byte_arr, format="JPEG", quality=95)
Expand Down Expand Up @@ -624,6 +632,13 @@

async def aclose(self) -> None:
await super().aclose()
# Tear down the AvatarRunner too: 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.
if self._avatar_runner is not None:
await self._avatar_runner.aclose()
self._avatar_runner = None
Comment on lines +639 to +641

@devin-ai-integration devin-ai-integration Bot Jun 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 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.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

if self._mode == "local" and utils.is_given(self._runtime) and self._runtime is not None:
self._runtime.cleanup()

Expand Down
Loading