Repository navigation
Conversation
Telethon fills a video's width, height and duration only when hachoir is installed, and hachoir is GPL-2.0 while this project is Apache-2.0, so every video went out as DocumentAttributeVideo(w=1, h=1, duration=0): portrait clips arrived squashed, durations showed 0:00. Parse the MP4/MOV container natively (mvhd for duration, tkhd for dimensions, applying the display matrix so 90-degree-rotated phone videos report portrait dimensions) and pass the attribute to Telethon, which overrides its own 1x1 default. Albums containing video are rebuilt as per-file InputMedia because Telethon's media-group branch never forwards caller attributes; image-only albums keep Telethon's own path. Video thumbnails are not attempted (they need a frame extractor); the preview stays blank until downloaded. Co-Authored-By: Claude Code <[email protected]>
axisrow
commented
Oct 24, 2026
axisrow
left a comment
axisrow
left a comment
Contributor
Author
There was a problem hiding this comment.
Review: PR #366 — real video dimensions and duration from send_file
Requesting changes: two bugs, each is an inline finding on telegram_mcp/tools/media.py.
Verified, no action needed:
- Premise checks out against Telethon 1.44.0 source: _send_album calls _file_to_media without attributes, so caller attributes really are dropped in media groups; rebuilding video albums as per-file InputMedia is justified. Image-only albums keep the native path.
- nosound_video=True matches what Telethon itself passes in _send_album and does not mute audible videos.
- Single-file path: attributes=[DocumentAttributeVideo] is merged per-class by utils.get_attributes, so DocumentAttributeFilename survives.
- Container walk is safe: top-level moov search, no infinite loop (size >= 8 enforced), tkhd v0/v1 offsets correct, 90/270-degree display-matrix swap correct; malformed inputs degrade to None and Telethon defaults.
Required changes:
- _parse_mvhd, version 1: mvhd v1 stores duration as a 64-bit field at offset 24; the >II unpack at offset 20 reads timescale plus only the high 32 bits of duration, which is zero for any real clip, so v1-mvhd videos still go out with duration=0 (the exact bug this PR fixes). Unpack >IQ from a 32-byte read with a len<32 check. The test helper _mvhd(version=1) packs the same wrong 28-byte layout and masks the bug; fix both together.
- _album_media_item: both InputMediaUploadedDocument branches omit DocumentAttributeFilename. Telethon always attaches one (utils.get_attributes), so in a rebuilt album the video and any non-image file arrive unnamed in clients. Add types.DocumentAttributeFilename(os.path.basename(path)) alongside the existing attributes.
Non-blocking: the commit message says the Telethon default without hachoir is duration=0; utils.get_attributes actually defaults to duration=1. Moot once finding 1 is fixed.
Verdict: changes_requested.
mvhd v1 stores duration as a 64-bit field at offset 24; the old >II read at 20 grabbed the timescale plus only the high 32 bits of the duration (zero for any real clip), so v1-mvhd videos still went out with duration=0. Unpack >IQ from a full 32-byte payload, and build the matching 32-byte v1 layout in the test helper that previously masked this (the 64-bit test now uses a duration beyond 2**32). Rebuilt album items also carry DocumentAttributeFilename like Telethon's own path-based uploads: without it, mixed-album videos and non-image files arrived unnamed. Co-Authored-By: Claude Code <[email protected]>
axisrow
commented
Oct 24, 2026
axisrow
left a comment
axisrow
left a comment
Contributor
Author
There was a problem hiding this comment.
Review: PR #366 — real video dimensions and duration from send_file (follow-up cd7a851)
Approving. Both findings from the previous round are fixed correctly:
- _parse_mvhd now unpacks >IQ from a 32-byte read with a len<32 check for version 1; the test fixture packs the real 32-byte v1 layout and uses duration 2**33 with timescale=1, so a parser reading only the high 32 bits would produce 0 and fail the assert; a truncated-v1 case was added to the malformed-input tests.
- _album_media_item attaches DocumentAttributeFilename in both InputMediaUploadedDocument branches, with tests asserting file_name for the video and for a non-image document in a mixed album.
Re-verified against the full diff to main: the single-file path and Telethon interplay are unchanged and still sound, image-only albums still take the native Telethon path, and nothing else moved in the follow-up commit.
Verdict: approved.
chigwell
approved these changes
Oct 24, 2026
This was referenced Oct 24, 2026
timurco
added a commit
to timurco/telegram-mcp
that referenced
this pull request
Oct 25, 2026
… and custom_emojis send_file and send_album accept the same parse_mode and custom_emojis as send_message, reply_to_message and edit_message. With parse_mode='html' a caption can carry <b>, <i>, links and <tg-emoji emoji-id="ID">, which Telethon parses itself. Without HTML, custom_emojis marks the standalone occurrences of each fallback emoji through the existing _plain_entities and is sent as formatting_entities over the raw caption. When neither argument is set nothing new reaches Telethon, so captions are still parsed as Markdown, exactly as before. parse_mode='plain' sends the caption verbatim, as in send_scheduled_message. custom_emojis together with parse_mode, rich modes, and custom_emojis without a caption are refused before anything is sent. Works for a single file and for a media group (the caption sits on its first item), with force_document, topic_id and schedule_date. An album that contains video goes out as per-file InputMedia (chigwell#366); the caption format is passed on that path as well.
axisrow
added a commit
to axisrow/telegram-mcp
that referenced
this pull request
Oct 30, 2026
Resolves the media.py conflict for the upstream side: it carries the same video-attributes work as fork #30 plus the chigwell#366 review follow-ups; the fork video code survives the auto-merge. Co-Authored-By: Claude Code <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #355 — videos sent through
send_filearrived withDocumentAttributeVideo(w=1, h=1, duration=0)and no thumbnail: portrait clips squashed, durations 0:00.Root cause
Telethon reads a video's width, height and duration only when
hachoiris installed, andhachoiris GPL-2.0 while this project is Apache-2.0, so it can't be added.send_filepasses noattributes, and Telethon's media-group branch never forwards caller attributes at all, so albums can't be fixed by passing kwargs either.The fix
struct+ seeking, no whole-file reads):mvhdfor duration,tkhdfor track dimensions, applying the display matrix so a 90-degree-rotated phone video stored as 1920x1080 reports 1080x1920 — the rotation hachoir gets wrong. Any malformed structure returnsNoneand the old behavior is kept (audio tracks are skipped by their zero dimensions; mvhd v0/v1 and 64-bit box sizes supported).send_file: for videos withforce_document=False, the parsed attribute is passed asattributes=[...]; Telethon overrides its own 1x1 default (attr_dict[type(a)] = a).InputMediaUploadedDocument/InputMediaUploadedPhotoand sent as a media group — the only way attributes survive. Image-only albums keep Telethon's own path untouched.Known limitation
Thumbnails are not attempted: extracting a frame needs an ffmpeg binary, which this project does not depend on, so the preview stays blank until the video is downloaded. Everything else from the issue (squashing, 0:00) is fixed for both single files and albums.
Tests
tests/test_send_file_video_meta.py: struct-built minimal MP4 fixtures — duration+dimensions, 90° rotation swap, identity matrix, 64-bit mvhd, five malformed/truncated containers returningNone(including one that reads across box boundaries — the parser clamps reads to the box end); wire-in tests: singlesend_filepasses the attribute, non-videos andforce_document=Truekeep the old path, album with video builds per-file media (video item carries the attribute andnosound_video=True, images becomeInputMediaUploadedPhoto), image-only albums unchanged. Full suite: 1498 passed;black --checkandflake8 --select=E501clean.🤖 Generated with Claude Code