Page MenuHomePhabricator

[native] Use getVideoInfo from MediaModule instead of ffmpeg
ClosedPublic

Authored by angelika on Thu, Mar 20, 4:34 AM.
Tags
None
Referenced Files
F5093037: D14471.id47482.diff
Sun, Mar 30, 10:56 AM
F5088905: D14471.id47547.diff
Sun, Mar 30, 12:34 AM
Unknown Object (File)
Sat, Mar 29, 10:10 AM
Unknown Object (File)
Fri, Mar 28, 3:00 PM
Unknown Object (File)
Thu, Mar 27, 5:47 PM
Unknown Object (File)
Wed, Mar 26, 1:42 PM
Unknown Object (File)
Wed, Mar 26, 10:13 AM
Unknown Object (File)
Wed, Mar 26, 2:42 AM
Subscribers

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

angelika held this revision as a draft.
lib/types/media-types.js
135 ↗(On Diff #47482)

I don't know what format is in the original ffmpeg implementation. It's not documented anywhere and when I've tested it the ffmpeg implementation didn't even return it. Because it's later compared to "mp4" to check if format is correct I'm just returning the file extension. We can run more complicated checks but I think it's good enough for our usecase.

native/media/video-utils.js
261 ↗(On Diff #47482)

The native code can return all those values depending on a platform and a video and they're all h264.

Looking at this diff, the thing you did with format seems right

This revision is now accepted and ready to land.Fri, Mar 21, 1:50 AM
native/media/video-utils.js
261 ↗(On Diff #47482)

If format wasn’t being set by ffmpeg, wouldn’t this check always fail? What happens if the check fails? If it’s not failing, then perhaps it’s worth understanding what ffmpeg is actually returning. Not sure it matters a lot though

262 ↗(On Diff #47482)

Should this and the above one be factored out to the root level to avoid redeclaring on every invocation?

angelika added inline comments.
native/media/video-utils.js
261 ↗(On Diff #47482)

I checked it and I must have printed a wrong thing because now I can see the format. But still it doesn't tell me much, for mp4 video I've got an array like [mov, mp4, ... a bunch of formats I don't know...]. So I'm leaving it as it is.