Page MenuHomePhabricator
Differential D14481

[native] Remove ffmpeg dependency
AcceptedPublic
Actions

Authored by angelika on Thu, Mar 20, 4:48 AM.
Tags
None
Referenced Files
F5025402: D14481.diff
Mon, Mar 24, 5:30 AM
F5024518: D14481.diff
Mon, Mar 24, 12:18 AM
F4976172: D14481.id47492.diff
Sun, Mar 23, 2:05 PM
F4974349: D14481.diff
Sun, Mar 23, 1:36 PM
Unknown Object (File)
Sun, Mar 23, 4:25 AM
Unknown Object (File)
Sun, Mar 23, 2:08 AM
Unknown Object (File)
Sat, Mar 22, 12:39 AM
Subscribers
ashoat
tomek

Details

Reviewers
bartek
Summary

https://linear.app/comm/issue/ENG-10314/migrate-comm-app-to-use-new-media-module

  • got rid of ffmpeg dependency
  • renamed ffmpeg.js to mediaProcessingQueue.js because the main feature of this file is the queue and wrapping functions from the media module
  • reanamed FFmpegStatistics and video_ffmpeg_transcode step

Depends on D14480

Test Plan

Compile the app and verify sending videos still work

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

angelika created this revision.Thu, Mar 20, 4:48 AM
angelika held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptThu, Mar 20, 4:48 AM
Harbormaster completed remote builds in B33788: Diff 47492.Thu, Mar 20, 5:22 AM
angelika edited the summary of this revision. (Show Details)Thu, Mar 20, 5:41 AM
angelika updated this revision to Diff 47503.Thu, Mar 20, 5:53 AM

Rebase

angelika published this revision for review.Thu, Mar 20, 5:57 AM
Harbormaster completed remote builds in B33799: Diff 47503.Thu, Mar 20, 6:11 AM
bartek accepted this revision.Fri, Mar 21, 8:57 AM

Wondering how much smaller our binaries will be without ffmpeg 😀

This revision is now accepted and ready to land.Fri, Mar 21, 8:57 AM
ashoat added inline comments.Fri, Mar 21, 4:13 PM
native/media/media-processing-queue.js
31 ↗(On Diff #47503)

Do we still need this synchronous queue? We previously had it because of limitations in ffmpeg-kit, but it reduces performance when a user uploads multiple media in a single message

bartek added inline comments.Sun, Mar 23, 11:55 PM
native/media/media-processing-queue.js
31 ↗(On Diff #47503)

I was thinking about this too, but I have a suspicion that mobile devices can struggle with processing multiple videos simultaneously and CPU performance will be a limiting factor

Revision Contents
Changeset List

PathSize
lib/
types/
6 lines
native/
android/
4 lines
ios/
Comm.xcodeproj/
16 lines
1 line
12 lines
media/
4 lines
15 lines
11 lines
1 line
5 lines

Diff 47492

View Options

lib/types/media-types.js

Loading...
View Options

native/android/build.gradle

Loading...
View Options

native/ios/Comm.xcodeproj/project.pbxproj

Loading...
View Options

native/ios/Podfile

Loading...
View Options

native/ios/Podfile.lock

Loading...
View Options

native/media/ffmpeg.js

Loading...
View Options

native/media/file-utils.js

Loading...
View Options

native/media/media-processing-queue.js

Loading...
View Options

native/media/video-utils.js

Loading...
View Options

native/package.json

Loading...
View Options

yarn.lock

Loading...