Skip to content

feat(rich-markdown): fill media attributes from ffprobe/ffmpeg - #21

Merged
popstas merged 19 commits into
masterfrom
feat/rich-markdown-media-probe
Aug 1, 2026
Merged

feat(rich-markdown): fill media attributes from ffprobe/ffmpeg#21
popstas merged 19 commits into
masterfrom
feat/rich-markdown-media-probe

Conversation

@popstas

@popstas popstas commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Fills Telegram media attributes ourselves from ffprobe/ffmpeg instead of relying on Telethon's metadata inference and the server's partial re-parse, so large videos stop rendering as empty rectangles and an animated .gif actually attaches.

Closes the two open rich_markdown items in docs/TODO.md.

Why

Telethon's utils.get_attributes() needs hachoir to read a media file. Without one it returns a stub DocumentAttributeVideo(duration=0, w=1, h=1) for every mp4 and no DocumentAttributeAudio at all. Telegram repairs the metadata by re-parsing the upload, but only below an undocumented size threshold — measured live 2026-07-29 (Saved Messages, msg 407429/407430), seven videos up to 6.30 MB came back with real duration, dimensions, thumbs=2 and supports_streaming=True, while three from 12.72 MB up kept duration=0, w=1, h=1, thumbs=None and rendered as an empty rectangle in the clients.

What changed

  • messages/media_probe.py (new, no Telethon imports, plain data out) — probe_media(), extract_thumbnail(), convert_gif_to_mp4(), ffprobe_available()/ffmpeg_available(). ffprobe/ffmpeg are optional external binaries, not pip dependencies: a failed probe is never an error (the pre-probe stub goes out with a WARNING naming the file), because a box without them must still be able to send. Every call is a blocking subprocess and runs through asyncio.to_thread, never on the event loop. A cover-art stream (disposition.attached_pic) is not a video stream — an .mp3 with artwork would otherwise be shaped like a video and take the artwork's dimensions.
  • _document_attributes() is now the single place a video/audio/animation attribute is built, replacing Telethon's stub rather than joining it (two DocumentAttributeVideo in one document is a malformed request).
  • Videos get an ffmpeg-generated preview frame (10% in — frame 0 of a real recording is often black) as thumb=. The missing preview is what makes the empty rectangle, so it is generated for every video rather than for large ones only.
  • An animated .gif is uploaded as a converted silent mp4 (temp file removed in a finally on every exit path), with the author's own <name>.mp4 written back as the filename attribute so the temp name never reaches the article. Since conversion is what makes the file attachable, a .gif with no ffmpeg on PATH is rejected by _validate_rich_files before the operation row is opened, leaving the idempotency key free for the retry after the install.
  • Every rich-media upload is now logged at INFO — uploading 34 files previously wrote nothing to the log at all, so a broken article was undiagnosable after the fact.

The markdown body is untouched by all of this: a .gif is still referenced as tg://video?id=… and media_kind() is unchanged. Conversion is purely about the upload's shape.

GIF wire facts, proven live

The plan assumed a .gif simply never attaches. scripts/spike_rich_gif.py (new, added in this branch) settled it against the real account on 2026-07-29 (Saved Messages, msg 407434-407437):

Upload Server response Article
98 KB image/gif transcoded server-sidevideo/mp4, 98 KB → 21 KB, Telegram's own DocumentAttributeVideo(w=320 h=240 duration=2.0) + Animated + thumbs=2, renamed loop.gif.mp4 attaches, real PageBlockVideo
21.2 MB image/gif not transcoded — keeps mime=image/gif, thumbs=None, DocumentAttributeAnimated dropped send fails RICH_MESSAGE_VIDEO_INVALID
21.2 MB → ffmpeg mp4 22.2 MB → 9.6 MB, real Video + Animated, thumbs=2 attaches, real PageBlockVideo

So a raw-gif upload is normalised by the server regardless of what we attach — but only below an undocumented threshold. Conversion is therefore unconditional: the small-gif case the server handles is not worth a second code path.

Testing

.venv/bin/pytest -q2201 passed (2168 before this branch), .venv/bin/ruff check src tests clean. All new tests use in-memory fakes and monkeypatched subprocess.run — no Telegram traffic and no real ffmpeg/ffprobe subprocess in the suite. The one live run was the GIF spike above, executed once under explicit authorization.

Docs

CLAUDE.md (the "Local media in an article is CLI-only" architecture bullet, plus a Common-commands entry for the new spike), README.md (including a note that the Docker image ships without ffmpeg), skills/telegram-assistant/SKILL.md (re-synced to ~/.claude/skills/), docs/TODO.md.

🤖 Generated with Claude Code

popstas and others added 19 commits July 29, 2026 01:56
… path

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A stale or ambiguous chat: rule aborted the whole index build, so every
gated command failed with the resolver's error. Catch EntityError per ref,
log access_rule_ref_unresolved and record it on Authorizer.unresolved_refs.
FloodWaitError still propagates.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
access check now echoes Authorizer.unresolved_refs so a stale chat: rule is
named in the payload instead of leaving the operator to grep stderr.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Skipping an unresolvable `chat:` ref dropped the rule's
delete_only_session_messages / edit_only_session_messages override along with
its grant. Those are restrictions, not capabilities, so dropping one *relaxed*
the policy: a permissive `all:` rule still granted DELETE while the targeted
hardening was gone, letting `messages delete` prune arbitrary messages in the
very chat the operator protected. A skipped `true` now becomes a global floor
(the hardened chat can no longer be identified), merged on top of the normal
chat > folder > all > default resolution by the same restrictive-`true`-wins
`_merge_only`; a skipped `false` contributes nothing.

Also bound the resolver cost: refs are resolved at most once per index build,
memoizing misses too, so N rules naming one stale ref no longer cost N
`iter_dialogs` title scans on every gated call. The memo covers the resolution
only — every naming rule still contributes its caps and overrides — and is
scoped to the build, so an aborted build retries cleanly.

`access check` dedups `unresolved_refs` by ref. Docs (CLAUDE.md, README,
SKILL.md) no longer claim skipping "only ever narrows rights".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Telegram transcodes a small GIF server-side (98 KB image/gif came back as
video/mp4 with its own DocumentAttributeVideo, DocumentAttributeAnimated and
thumbs=2), but stops above a size threshold: a 21.2 MB image/gif upload kept
its mime, got thumbs=None, had DocumentAttributeAnimated dropped, and the
article send failed with RICH_MESSAGE_VIDEO_INVALID. The ffmpeg-converted mp4
attached in both cases. This is the evidence behind converting every .gif
client-side rather than relying on the server.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ected

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The git-cliff pre-commit hook regenerates CHANGELOG.md from git log before
the pending commit exists, so it always lags one commit behind. Catch it up
so every feature commit on this branch is listed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s original name

- convert_gif_to_mp4's docstring claimed a raw image/gif never attaches to an
  article at all; the 2026-07-29 spike proved a small gif (98 KB) does attach
  via server-side transcoding and only a large one (21.2 MB) fails with
  RICH_MESSAGE_VIDEO_INVALID. Reworded to state the real reason conversion is
  unconditional (the transcode threshold is undocumented), and fixed the same
  stale sentence in two test docstrings.
- _document_attributes/_upload_video_thumbnail now take a keyword-only
  log_path, defaulting to the functional path they already receive; the
  _upload_rich_files call sites pass rich_file.path so a converted gif's
  probe/thumbnail warnings name the author's loop.gif instead of the temp mp4
  that is unlinked by the time anyone reads the log. The functional path
  (what gets probed/thumbnailed/uploaded) is untouched.
- Split the video branch's warning reason into no_probe/no_video_stream/
  no_dimensions instead of collapsing the last two into no_video_stream.
- Stubbed probe_media/extract_thumbnail in three rich-send tests that
  previously shelled out to real ffprobe/ffmpeg against garbage bytes;
  assertions unchanged.
- Added a test asserting a converted gif's probe-failure warning names
  loop.gif rather than the temp path.
- Added a comment recording that a MediaConversionError raised after the
  operation row opens is intentionally left as a terminal failed state.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
messages send --rich-markdown with local media degrades inside the
container (a .gif hard-rejects, videos may render as an empty rectangle)
since the image is built to run the API only and never touches local media.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@popstas
popstas merged commit 215429b into master Aug 1, 2026
2 checks passed
@popstas
popstas deleted the feat/rich-markdown-media-probe branch August 1, 2026 16:47
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.

1 participant