Skip to content

fix(telegram): pass width/height/duration on sendVideo so portrait videos keep their aspect ratio - #73701

Open
zzzmeu wants to merge 1 commit into
NousResearch:mainfrom
zzzmeu:fix/telegram-sendvideo-dimensions
Open

fix(telegram): pass width/height/duration on sendVideo so portrait videos keep their aspect ratio#73701
zzzmeu wants to merge 1 commit into
NousResearch:mainfrom
zzzmeu:fix/telegram-sendvideo-dimensions

Conversation

@zzzmeu

@zzzmeu zzzmeu commented Jul 29, 2026

Copy link
Copy Markdown

Problem

The Telegram adapter's send_video omits the optional width/height/duration parameters on sendVideo. When they are missing, Telegram's server guesses the aspect ratio from the uploaded file — and sometimes gives up and renders the video in a square player. We hit this deterministically with portrait 9:16 (1080x1920) H.264 MP4s: some uploads displayed correctly, others (byte-structure identical: same encoder, same faststart layout, same SAR/DAR) were shown square on every resend. The agent's own ffprobe checks kept truthfully reporting 1080x1920, which made the failure confusing to debug from the chat side.

Fix

Probe the real dimensions and duration with ffprobe right before sending and pass them explicitly — Telegram then renders every video exactly as encoded, on every client.

  • Best-effort: if ffprobe is not installed, the probe returns {} and the previous behavior is preserved unchanged.
  • Looks for ffprobe on PATH plus the common Homebrew (/opt/homebrew/bin) and /usr/local/bin locations, since launchd/systemd service environments often have a minimal PATH.
  • 10s subprocess timeout; any probe failure degrades silently to the old behavior.

Testing

  • Verified the probe returns {'width': 1080, 'height': 1920, 'duration': 18} for the affected files.
  • With the patch applied, the previously-square portrait video renders 9:16 in Telegram (iOS + macOS clients).
  • ast.parse clean; no new dependencies.

…deos keep their aspect ratio

Telegram's Bot API treats width/height/duration as optional on sendVideo,
but when omitted the server guesses the aspect ratio from the file and
sometimes falls back to a square player - portrait 9:16 videos are the
usual casualty (observed deterministically on some uploads while
byte-identical-structure files displayed fine). Probe the real dimensions
with ffprobe when available (PATH, Homebrew, /usr/local) and pass them
explicitly; without ffprobe the previous behavior is preserved.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 29, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: this is a metadata-only repair in the #61569 Telegram video cluster. Open #61570 is the broader current-path repair with thumbnail, streaming, local-mode handling, and tests; #21327 targets the retired gateway/platforms path.

@teknium1 teknium1 left a comment

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.

Thanks for the focused repair. The premise remains current: TelegramAdapter.send_video() on main omits video dimensions and duration at plugins/platforms/telegram/adapter.py:7193-7200.

Problems

  • The new direct probe call at plugins/platforms/telegram/adapter.py:7224 runs a blocking subprocess.run(..., timeout=10) from an async send path. That can stall the Telegram event loop for up to ten seconds. The analogous existing voice probe is deliberately offloaded with await asyncio.to_thread(...) at plugins/platforms/telegram/adapter.py:6753-6755.
  • This PR changes only plugins/platforms/telegram/adapter.py; current coverage at tests/gateway/test_telegram_documents.py:530-546 only asserts that send_video was called, so metadata forwarding and probe-failure fallback are untested.

Suggested changes

  • Offload _probe_video_meta with asyncio.to_thread.
  • Add hermetic tests for forwarded width/height/duration and the {} fallback.

Automated hermes-sweeper review.

@@ -7176,6 +7224,7 @@ async def send_video(
reply_to_message_id=reply_to_id,

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.

_probe_video_meta() calls blocking subprocess.run(..., timeout=10), so invoking it directly in this async method can stall the Telegram event loop for ten seconds. Please use await asyncio.to_thread(self._probe_video_meta, video_path), matching the existing voice-duration probe at current main adapter.py:6753-6755.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two open PRs address Telegram portrait videos being rendered as square by supplying explicit video metadata. #21327 adds dimensions, duration, streaming, and thumbnail generation on the retired gateway path, while #73701 applies the narrower dimensions-and-duration repair to the current plugin adapter and preserves its retry, routing, and notification flow.

Related pull requests

  • #21327 duplicate — (+135/-7) — duplicate: The diff probes width, height, and duration and adds streaming and thumbnail handling, but it modifies the retired gateway/platforms/telegram.py path and runs ffprobe/ffmpeg synchronously inside async send_video(); its tests also mock the probe boundary. Despite the keep_open review on #21327, close as duplicate of #73701 because the shared metadata repair belongs on the current plugin path; its extra thumbnail work would require a deliberate non-blocking port and real probe-boundary tests.
  • #73701 related — (+50/-0) — keep open with a salvage path: The diff updates plugins/platforms/telegram/adapter.py and forwards probed width, height, and duration through the existing DM-topic retry and notification path, directly addressing Telegram's incorrect aspect-ratio inference. Consistent with the keep_open review on #73701, the author should offload the blocking probe with asyncio.to_thread and add hermetic metadata-forwarding and probe-failure fallback tests.

Duplicates

#21327 and #73701 substantially duplicate the ffprobe-based width/height/duration repair; #21327 additionally attempts thumbnail generation and supports_streaming, but only on the retired adapter path.

Suggested consolidation

Keep #73701 open with a salvage path: retain its current-path metadata forwarding, move _probe_video_meta off the event loop, and add success and fallback coverage; also compare its scope with the contributor-identified broader repair in #61570 before treating it as the cluster-wide solution. Close #21327 as duplicate of #73701 because its shared fix targets the retired path; any valuable thumbnail or supports_streaming behavior should be split out and ported deliberately rather than preserving that diff.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup21327 ["PRs duplicating each other"]
        P21327["PR #21327 (open)"]
        P73701["PR #73701 (open)"]
    end
    class P21327 open
    class P73701 open
    class P73701 target
    click P21327 "https://github.com/NousResearch/hermes-agent/pull/21327"
    click P73701 "https://github.com/NousResearch/hermes-agent/pull/73701"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 11 kB of PR diffs, 3 kB of issue/PR text, 3 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants