You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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
Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch. Learn more about Cursor Agents
Renames the video playback component to MediaPlayer with prop updates and a backward-compatible alias. Expands supported MIME types to include several audio types and maps them to “video” handling. Tightens video-processing conditions to require video MIME types. Hardens video analytics validation and aggregation. Minor cosmetic changes to imports and a trailing newline.
Changes
Cohort / File(s)
Summary
Player rename and props alignment components/view/viewer/video-player.tsx, components/view/viewer/video-viewer.tsx
Renamed VideoPlayer to MediaPlayer; prop videoSrc → mediaSrc; default preventDownload set to true; maintained alias export const VideoPlayer = MediaPlayer; updated imports/usages in VideoViewer.
MIME support expansion lib/constants.ts, lib/utils/get-content-type.ts, lib/utils/get-file-icon.tsx
Added audio MIME types: audio/mp4, audio/x-m4a, audio/m4a, audio/mpeg. Classified as “video” in getSupportedContentType; extensions mapped (m4a, mp3); icons updated to use VideoIcon; included in supported/accepted types.
Video processing gate hardening lib/api/documents/process-document.ts, pages/api/.../versions/index.ts
Gated processing to type === "video" with contentType?.startsWith("video/") and exclusion of video/mp4; avoids triggering when contentType is missing or non-video.
Analytics validation and aggregation pages/api/.../video-analytics.ts
Added event schema validation, per-second aggregation safeguards, unique viewer tracking per second, defensive metrics (avg duration, first/last viewed), and response validation for analytics fetch.
Added trailing newline; reordered some imports without logic changes.
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client
participant API as Versions API
participant Proc as Video Processor
Client->>API: Upload document version (type, contentType, url)
alt contentType startsWith("video/") && type === "video" && contentType != "video/mp4"
API->>Proc: trigger processVideo(url, teamId, documentVersionId, ...)
Note right of Proc: Starts async processing pipeline
else Not a qualifying video
API-->>Client: Skip video processing path
end
Loading
sequenceDiagram
autonumber
participant Client
participant API as Video Analytics API
participant TB as Tinybird
Client->>API: GET /video-analytics?docId=...&videoLength=...
API->>TB: Query analytics data
TB-->>API: data[]
alt data is valid array
API->>API: Validate and filter events (types, times, view_id)
API->>API: Aggregate per-second buckets and unique viewers
API-->>Client: Aggregated metrics and distributions
else invalid/missing data
API->>API: Log error
API-->>Client: 500 with error message
end
fix: don't optimized mp4 files #1756 — Adjusts video-processing gating in process-document.ts and versions API; closely related to the refined gating here.
fix: codeql #1780 — Strengthens API input validation patterns similar in spirit to the analytics validation added here.
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.
You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name
Status
Explanation
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check
✅ Passed
The title "Add m4a and mp3 audio support" accurately and concisely describes the PR's primary change—adding M4A/MP3 MIME types and related support (constants, content-type handling, icons, and viewer/analytics)—and is clear and scan-friendly. Other edits in the changeset (MediaPlayer rename, import reordering, defensive checks) are secondary and do not make the title misleading or irrelevant.
✨ Finishing touches
📝 Generate Docstrings
🧪 Generate unit tests
Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch cursor/add-m4a-and-mp3-audio-support-1c1b
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
78-85: Unique views should be computed from validated events (and skip falsy view_id).
Counting from all events can inflate unique_views and avg_view_duration denominator with invalid/undefined view_ids.
Apply this diff:
- const uniqueViewIds = new Set(events.map((e) => e.view_id));+ const uniqueViewIds = new Set(+ validEvents.map((e) => e.view_id).filter((v): v is string => Boolean(v))+ );
Also applies to: 144-157
lib/constants.ts (1)
67-102: Map audio/ to the correct size limit (treat as video or add a dedicated audio limit)*
getFileSizeLimit in lib/utils/get-file-size-limits.ts currently falls through to limits.document for audio/*; handle audio explicitly — either add an "audio" limit to FileSizeLimits/getFileSizeLimits or at minimum add if (fileType.startsWith("audio/")) return limits.video;. Location: lib/utils/get-file-size-limits.ts → getFileSizeLimit.
components/upload-zone.tsx (1)
595-608: Populate audio extension arrays in lib/constants.ts to restore extension-based MIME resolution.
upload-zone resolves MIME by matching file extensions against acceptableDropZoneFileTypes (components/upload-zone.tsx lines 593–601); lib/constants.ts currently has empty arrays for "audio/mp4", "audio/x-m4a", "audio/m4a", and "audio/mpeg" (lib/constants.ts lines 145–148), so .m4a/.mp3 dropped from folders won't match — add the appropriate extensions (e.g. [".m4a", ".mp3"]).
🧹 Nitpick comments (13)
components/upload-zone.tsx (2)
720-723: Update hint text to reflect new audio support (.m4a, .mp3).
The drop hint omits audio types now accepted for paid plans.
59-61: Include all m4a MIME variants for consistent icon rendering.
audio/x-m4a and audio/m4a aren’t covered; they’ll fall to default FileIcon.
Apply this diff:
case "video/x-msvideo":
case "video":
+ case "audio/x-m4a":+ case "audio/m4a":
case "audio/mp4":
case "audio/mpeg":
return <VideoIcon className={className} isLight={isLight} />;
components/welcome/special-upload.tsx (1)
94-96: Clarify unsupported-format message (now misleading if media is allowed here).
If this flow can accept any supported types, broaden the copy. If it’s intentionally docs-only, ignore.
Apply this diff:
- "Unsupported file format. Please upload a PDF or Excel file.",+ "Unsupported file format.",
lib/constants.ts (1)
145-148: Populate extensions for new audio MIME types to fix folder-upload mime resolution.
Empty arrays break the extension-based fix in upload-zone for files with type "" (Firefox).
This achieves reuse of the player/analytics pipeline for m4a/mp3.
If you want broader real‑world coverage without adding many cases, consider also accepting common non‑standard aliases like "audio/mp3" and "audio/x-mp3".
105-110: Add content‑type normalization to handle parameters/case
Strict equality can miss values like audio/mpeg; charset=binary or mixed case. Normalize once per function.
echo
echo "---- rg for MediaPlayerProps / MediaPlayer type ----"
rg -nP --hidden --no-ignore 'MediaPlayerProps\b|type\s+MediaPlayerProps\b|interface\s+MediaPlayerProps\b' -S || true
echo
echo "---- Show components/view/viewer/video-player.tsx (if exists) ----"
if [ -f components/view/viewer/video-player.tsx ]; then
sed -n '1,400p' components/view/viewer/video-player.tsx
else
echo "file not found: components/view/viewer/video-player.tsx"
fi
echo
echo "---- Show components/view/viewer/video-viewer.tsx (if exists) ----"
if [ -f components/view/viewer/video-viewer.tsx ]; then
sed -n '1,400p' components/view/viewer/video-viewer.tsx
else
echo "file not found: components/view/viewer/video-viewer.tsx"
fi
</blockquote></details>
<details>
<summary>components/view/viewer/video-viewer.tsx (1)</summary><blockquote>
`10-10`: **Import swap to MediaPlayer looks good — verified.**
No remaining references to VideoPlayer or videoSrc; video-player.tsx still re-exports VideoPlayer for backward compatibility (components/view/viewer/video-player.tsx:76–77).
</blockquote></details>
</blockquote></details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->
The reason will be displayed to describe this comment to others. Learn more.
⚠️ Potential issue
Alias isn’t prop‑compatible; accept both mediaSrc and legacy videoSrc.
The alias export alone doesn’t preserve the old prop name; existing usages will break. Support both to truly be backward‑compatible.
Apply this diff:
-interface MediaPlayerProps {- mediaSrc: string;+interface MediaPlayerProps {+ mediaSrc?: string;+ // Back-compat with old VideoPlayer prop+ videoSrc?: string;
onError?: (error: Error) => void;
controls?: boolean;
className?: string;
preventDownload?: boolean;
}
@@
- mediaSrc,+ mediaSrc,+ videoSrc,
onError,
controls = true,
className = "",
preventDownload = true,
},
@@
- src={mediaSrc}+ src={mediaSrc ?? videoSrc ?? ""}
>
- <source src={mediaSrc} />+ <source src={mediaSrc ?? videoSrc ?? ""} />
Your browser does not support the media tag.
</video>
@@
// Keep the old VideoPlayer export for backward compatibility
export const VideoPlayer = MediaPlayer;
Also applies to: 23-29, 64-68, 76-78
🤖 Prompt for AI Agents
In components/view/viewer/video-player.tsx around lines 11-17 (and similarly at
23-29, 64-68, 76-78), the new prop alias only exports mediaSrc and will break
existing callers using the legacy videoSrc prop; update the component props and
runtime handling to accept both names by adding videoSrc?: string to the
interface, normalize inside the component (const src = mediaSrc ?? videoSrc) and
use src for rendering and error callbacks, and update any prop-types or exported
types so both props are accepted and documented; ensure TypeScript types mark
one as optional and preserve onError/controls/className/preventDownload
behavior.
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
None yet
2 participants
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Add M4A and MP3 audio file support, leveraging existing video infrastructure for playback and analytics.
Slack Thread
Summary by CodeRabbit
New Features
Bug Fixes