[Feature] Video optimization step 1#6088
Conversation
…r-mobile) to dependencies. Also added an exception to the main manifest file for it, since the library is built for Android v18+, instead our min SDK is 16.
Enable video optimization flag for Alpha flavour, and for debug build on all flavours.
…ss-Android into feature/video-compression-framework
Also improved the error message shown when we know the video cannot be optimized.
| } | ||
| } | ||
|
|
||
| private class VideoOptimizationProgressListener implements org.m4m.IProgressListener { |
There was a problem hiding this comment.
What do you think about moving all the video optimization code to a separate class? My concern is that EditPostActivity is getting very large and unwieldy.
There was a problem hiding this comment.
Good call, will move this logic to WPVideoUtils.
| @@ -0,0 +1,111 @@ | |||
| package org.wordpress.android.util; | |||
There was a problem hiding this comment.
This class would benefit from much more commenting. That would make it much easier for another dev to work on it.
|
|
||
| <!-- Video processing --> | ||
| <string name="video_optimization_in_progress">Optimizing your video…</string> | ||
| <string name="video_optimization_cant_optimize">Sorry, we can\'t optimize this video. Using the original file.</string> |
There was a problem hiding this comment.
We don't appear to use video_optimization_cant_optimize anywhere.
There was a problem hiding this comment.
Right, it's not used anymore. Forgot to remove it.
| dismissProgressDialog(pd); | ||
| Context context = mWeakContext.get(); | ||
| if (context != null) { | ||
| ToastUtils.showToast(context, R.string.video_optimization_generic_error_message, Duration.LONG); |
There was a problem hiding this comment.
I'm wondering if this toast is necessary. The user can't do anything about it, and I can't think of why they'd need to know this information.
There was a problem hiding this comment.
It's there just to inform our alpha testers of the error occurred compressing the video, but I agree we can remove it and add a Crashlytics log instead to gather stats about compressions problems with videos.
| private static final int AUDIO_OUTPUT_BIT_RATE = 96 * 1024; | ||
|
|
||
| public static MediaComposer getVideoOptimizationComposer(Context ctx, String inputFile, String outFile, org.m4m.IProgressListener listener) { | ||
| if (ctx == null || inputFile == null || listener == null) { |
There was a problem hiding this comment.
We could skip these null checks and simply make the parameters @NonNull.
…ss-Android into feature/video-compression-framework # Conflicts: # WordPress/src/main/res/values/strings.xml
…one when Async will be in place.
…ss-Android into feature/video-compression-framework
|
@nbradbury - Ready for another pass! |
|
Looks good! |
This PR adds and uses the Media for Mobile library in order to compress and resize video files before uploading to the server.
We need to test video re-encoding capabilities on a large set of devices before shipping it to our end-users, so basically the plan to test this new feature/library is the following:
Our end users will not see this feature until we decide is stable enough, and we've completed the integration of it in Settings screen. I suppose there will be a new setting for it.
Now the process of re-encoding a video is a blocking task, there is a progress dialog in place that is shown until the video is processed. Then, the resulting video is added to the visual editor and the upload starts.
In our final implementation it's better to integrate video re-encoding in the async branch only. Once we've tested that the library works fine.
Videos are re-encoded to 720p (if they're larger) 30 frame per sec, 3000 KBytes rate.
Next steps (step 2) is to start working in the settings screen, and provide the ability to turn this feature ON/OFF (and select different bitrate?).