Repository navigation
feat(obs): carry a video's length through the E2EE upload - #240
Conversation
uploadMediaByE2EE builds the message's contentMetadata itself and had no way to carry a duration, so an E2EE video went out with no DURATION and LINE clients rendered it as a 0:00 clip. On this path obs only ever sees the encrypted blob, so it cannot read the length out of the container the way it does for a plain uploadObjTalk (which already sends an obs "duration" param); the value has to travel in contentMetadata instead. Video only: nothing in the thrift types says LINE reads the same key for a voice message, so audio is left alone until that is confirmed. A non-finite or non-positive duration is dropped rather than sent as garbage. (cherry picked from commit a041d4a)
There was a problem hiding this comment.
🟡 Changes recommended
The new DURATION guard can still emit "0" when durationMs is a small positive value that rounds to zero, which undermines the intended “drop non-positive durations” behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes missing video duration metadata for the E2EE upload path in @evex/linejs by adding an optional durationMs parameter to LineObs.uploadMediaByE2EE and threading it into the outgoing talk.sendMessage contentMetadata as DURATION for videos, with accompanying unit tests.
Changes:
- Add
durationMs?: numbertouploadMediaByE2EEoptions and include a guardedDURATIONentry incontentMetadataforoType === "video". - Add tests asserting
DURATIONis set for videos with a valid duration, and absent otherwise (omitted, non-video, non-finite, or non-positive).
File summaries
| File | Description |
|---|---|
| packages/linejs/base/obs/mod.ts | Adds optional durationMs and spreads DURATION into E2EE message contentMetadata for videos. |
| packages/linejs/base/obs/upload_e2ee.test.ts | Adds coverage to verify DURATION behavior across valid/invalid/omitted duration inputs and non-video types. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Reviewed both changed files and the adjacent upload/encryption/sendMessage paths. Follow-up 2bbc4a7 fixes the rounded-zero bug (also noted by Copilot): 0.1ms previously emitted DURATION="0". Validation now runs on the rounded positive safe integer, also excluding unsafe/scientific-notation values. Added four test groups for boundaries, non-video metadata parity (audio/file/image/GIF), and video preview key reuse plus complete message metadata. Validation: deno test -A = 351 passed / 0 failed; touched-file deno lint and fmt passed; linejs and linejs-types JSR dry-runs passed. Existing Node transport test failed locally twice: RPC cancellation produced TypeError after ~31s instead of AbortError, while PUSH passed. The test and its runtime import (base/core/node_fetch.ts) are unchanged from main. Waiting for updated CI, including its Node transport step, before merge. Protocol evidence is limited: LINE public Messaging API documentation uses milliseconds for received video duration (https://developers.line.biz/en/reference/messaging-api/nojs/), and existing independent client code uses DURATION for video (https://github.com/fadhiilrachman/line-py/blob/master/linepy/talk.py). This is corroboration, not a live E2EE compatibility test. No live LINE account was used; audio remains intentionally unchanged. The option is caller-supplied and does not infer duration from the container. Target patch release: v3.4.2. |
|
Thanks for pushing 2bbc4a7 onto the branch — adopted as-is, nothing of mine on top. Verified locally on that commit: One thing from the description still open, whenever convenient: audio. The E2EE path still sends no length for a voice message, because nothing in the tree confirms the key name for one. If |
|
Crossed with your comment — the audio question is answered, thanks. The Messaging API duration units plus line-py using |
Summary
A video uploaded over E2EE arrives on LINE clients with no length — the bubble renders as a 0:00 clip — while the same file sent through the plain upload path shows its real duration.
uploadMediaByE2EEhas no way to carry one.The two paths differ in who builds the message.
uploadObjTalk(packages/linejs/base/obs/mod.ts:177-237) never builds one: it posts the object withoid: "reqseq"and lets the server create the message from the obs params, and it already sends the length as an obs param —(
:221-225), whichSquareChat#sendVideo/#sendAudiothread through (packages/linejs/client/features/square/mod.ts:131-164).uploadMediaByE2EEbuilds the message'scontentMetadataitself (packages/linejs/base/obs/mod.ts:415-437) and that literal has no duration key at all:Nothing downstream can fill it in, either: on this path obs only ever receives the encrypted blob (
:368-380uploadsencryptedData), so there is no container for the server to read a length out of the way there is for a plain upload. The value has to travel incontentMetadata.Fix. An optional
durationMson the options object, spread intocontentMetadataasDURATIONwhen — and only when —oType === "video":Additive throughout: the option is optional, every existing call site behaves exactly as it did, and
packages/typesneeds nothing —contentMetadatais aRecord<string, string>on the wire and already accepts the key.0, a negative,NaNandInfinityare dropped rather than sent as garbage, since a bogus length renders worse than none.Two details worth flagging for review:
Record<string, string>annotation ondurationMetadatais load-bearing, not decoration. Without it TS widens the ternary to{ DURATION: string } | { DURATION?: undefined }, and the second member is not assignable to the index signature onsendMessage'scontentMetadata(packages/linejs/base/service/talk/mod.ts:96) —deno checkfails with TS2322.DURATIONis the key a LINE video message carries for its length, in milliseconds. It cannot be cross-checked inside this repository: nothing inpackages/typesmodelscontentMetadata, which is an untyped string map on the wire (grep -rn DURATION packages/returns nothing outside this change and the unrelatedparam.durationabove). If you know the key by another name, it is one string to change.Audio is deliberately left out. For the same reason: the tree gives no evidence for what key a voice message uses, and
DURATIONfor audio would be a guess written into the protocol layer.oType === "audio"is therefore untouched and still sends no length over E2EE. If you can confirm the voice key, it is a one-token change to that guard plus one test case — happy to add it here or as a follow-up, whichever you prefer.Testing
Three cases added to
packages/linejs/base/obs/upload_e2ee.test.ts, built on that file's existingfakeClient()/makeObs()fixtures plus a smallsentMetadata()helper that asserts exactly onetalk.sendMessageand returns itscontentMetadata:durationMs: 4200.4→DURATION === "4200"(milliseconds, rounded).durationMs→ noDURATION; and an image withdurationMs: 4200→ noDURATIONeither, covering the video-only guard.0,-1,NaNandInfinityon a video → noDURATION.Case 1 doubles as the negative control: commenting the
...durationMetadataspread back out fails it (5 passed | 1 failed) and leaves the other five obs cases green, so it is testing the new line rather than the fixture.cd packages/linejs && deno test -A— 303 passed, 0 failed, twice in a row.deno fmt --checkanddeno linton both touched files, anddeno check packages/linejs/client/mod.ts, are clean.client/features/call/stun.test.tsneeds a live UDP round trip tostun.l.google.com; it passed on both runs here, but it has flaked on this machine before and is untouched by this change.