Using stories lib in video compression scenario by develric · Pull Request #14256 · wordpress-mobile/WordPress-Android

develric · 2021-03-15T02:12:26Z

This PR aims to use the stories library to manage the video compression/optimization scenario in WPAndroid.

Companion stories PR in Automattic/stories-android#652

NOTE: the feature flag currently is disabled, and it covers the video compression only part. The stories lib modifications are not feature flagged so we need to test them well to avoid regressions or issues since we will not be able to remote disable the changes. Once we are ok with testing of stories and video compression we can decide to merge this PR and create another dedicated PR to activate the flag for video compression in production. We will keep the flag so that we can monitor the release and eventually disable remotely the video compression feature, falling back to the original implementation of the video compression.

To test

The feature is behind feature flag, you need to activate the MP4_COMPOSER_VIDEO_OPTIMIZATION in App Settings > Test feature configuration, then restart the app to make the flag active.

Also please turn on Optimize Videos option in App Settings.

Stories use case

First things first 😄 , we need to test the stories scenarios to make sure no issues are introduced by the changes

Video compression ON

Make sure you activated Optimize Videos and MP4_COMPOSER_VIDEO_OPTIMIZATION
In the Media library and in the Editor try to upload various types and sizes of videos from device or google drive
Check the video is uploaded correctly
Check both audio and video playback is as expected
in the app storage space > cache folder (on the device or emulator) check you get the compressed copy named like wp-<auto created unique id>.<video format extension>

Video compression OFF

Make sure you de-activated Optimize Videos and MP4_COMPOSER_VIDEO_OPTIMIZATION is ON
In the Media library and in the Editor try to upload various types and sizes of videos from device or google drive
Check the video is uploaded correctly
Publish the post
Check both audio and video playback is as expected
in the app storage space > cache folder (on the device or emulator) check you DO NOT get the compressed copy named like wp-<auto created unique id>.<video format extension>

Check everything works if feature flag is off

Test stories and video compression with feature flag off

PR submission checklist:

I have considered adding unit tests where possible.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

peril-wordpress-mobile · 2021-03-15T02:12:32Z

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

peril-wordpress-mobile · 2021-03-15T02:22:24Z

You can test the changes on this Pull Request by downloading the APK here.

mzorz · 2021-05-14T07:36:33Z

WordPress/src/main/java/org/wordpress/android/ui/uploads/VideoOptimizerBase.java

+        String strSavingsKb = new DecimalFormat("0.00").format(savingsKb).concat("KB");
+
+        // make sure the resulting file is smaller than the original
+        if (savings <= 0) {


Would it be nice to delete the produced file in mOutputPath, once we realize we're not going to be using it after all?

Good point 👍 ! if you agree I created a dedicated issue for this here

mzorz · 2021-05-14T07:37:28Z

WordPress/src/main/java/org/wordpress/android/ui/uploads/VideoOptimizerProvider.kt

+package org.wordpress.android.ui.uploads
+
+// TODO: move here the VideoOptimizationListener interface and ProgressEvent. Leaving them in their original
+// position for now to not change the files as they are


👍 agreed, it might be good to have them together

Moved interfaces and data classe used by them to VideoOptimizerContracts.

mzorz · 2021-05-14T08:35:45Z

WordPress/src/main/java/org/wordpress/android/util/WPVideoUtils.java

+                VideoFormatMimeType.AVC,
+                bitrate * 1024,
+                2,
+                96 * 1024,


Could we maybe move these to constants and add a comment next to each one, explaining the reasoning why these values are passed like this?
Also wondering, could it be possible that we need to pass any other values or will this work universally?

Not a great study behind that tbh 😄 ....I basically just tried to use the set of parameters that were used (as a fixed set like here) in the previous implementation. Added a comment in the code that hope can help to clarify, let me know 🙇

thanks for your answer! sounds good 👍

mzorz

Nice work here @develric 👏

Left a few comments but overall nothing really pressing to change. Already reviewed and approved the Stories PR Automattic/stories-android#652 so feel free to tackle the comments here and merge after everything is up to date with develop.
Ping me once it's updated (or let me know and I can open a new PR to update with develop, against your branch), so I can give it another good pass on both PRs 👍

# Conflicts: # WordPress/build.gradle

…ession parameters.

develric · 2021-05-21T10:29:41Z

Hi @mzorz 👋 , merged from develop and followed up to your feedbacks, let me know wdyt 🙇

mzorz

Awesome job @develric, took the liberty to update the stories library hash after merging PR Automattic/stories-android#652 first in b56413c, leaving the same summary of things tested here as well so it's easier to track later.

Pixel 4a:

tried with videos on device of a size smaller than the optimize target, verified the original file is returned
tried with videos on device of a size larger than the optimize target, verified the conversion happens
tried with videos on google drive, also worked fine for both cases above
Also tested the optimizer gets called as well for videos being added to Stories (kind of schroedinger cat), and verified it plays along well with the web player (i.e. when you upload a video and add text to a video story slide, the mp4composer will be used once to process the new video, then called another time for optimizing)
Also tested turning optimization off, verified it doesn't go through the mp4composer optimization process.

Samsung J2:

tried with videos on device of a size smaller than the optimize target, verified the original file is returned
tried with videos on device of a size larger than the optimize target, verified the conversion happens
Also tested turning optimization off, verified it doesn't go through the mp4composer optimization process.

peril-wordpress-mobile · 2021-05-24T09:13:30Z

	Warnings
⚠️	PR is not assigned to a milestone.

Generated by 🚫 dangerJS

develric added 6 commits March 15, 2021 02:55

Adding property to track m4m optimizer.

4f9b6d2

Creating Optimizer hierarchy for m4m and mp4composer libs

61ebbbc

Adding feature flag

1b000f8

Adding getVideoOptimizationComposer for mp4composer.

bb6d107

Feature flagging which optimizer to call for video optimization.

865fd07

Updating stories branch reference.

5c24534

develric added [Type] Enhancement Media labels Mar 15, 2021

develric requested review from daniloercoli and mzorz March 15, 2021 02:14

develric added 2 commits March 15, 2021 12:54

Updating stories branch reference.

1ee4bf4

Fixing linter.

4bf33da

mzorz reviewed May 14, 2021

View reviewed changes

mzorz approved these changes May 14, 2021

View reviewed changes

develric added 3 commits May 20, 2021 15:06

Updating stories reference.

964f040

Merge branch 'develop' into try/using-lib-in-video-compression-scenario

3835e91

# Conflicts: # WordPress/build.gradle

Updating stories lib reference

5812f72

develric mentioned this pull request May 20, 2021

When a video compression is not saving we should delete the output file. #14687

Open

develric added 4 commits May 20, 2021 23:56

Moving optimizer interfaces into VideoOptimizerContracts.

1f99853

Fixing lint.

783c0d2

Removing unused imports.

c9249e9

Adding a comment to clarify the rationale behind selected video compr…

cbf0a98

…ession parameters.

develric marked this pull request as ready for review May 21, 2021 10:03

mzorz self-assigned this May 24, 2021

updated stories library to latest develop

b56413c

mzorz approved these changes May 24, 2021

View reviewed changes

mzorz enabled auto-merge May 24, 2021 09:13

mzorz merged commit 4e1c718 into develop May 24, 2021

mzorz deleted the try/using-lib-in-video-compression-scenario branch May 24, 2021 09:20

mzorz added this to the 17.5 milestone May 24, 2021

ParaskP7 mentioned this pull request Feb 7, 2023

[Dependency Updates] Reconfigure Media-for-Mobile Dependency #17903

Merged

3 tasks

Conversation

develric commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To test

Stories use case

Video compression ON

Video compression OFF

Check everything works if feature flag is off

Uh oh!

peril-wordpress-mobile bot commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

peril-wordpress-mobile bot commented Mar 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mzorz May 14, 2021

Choose a reason for hiding this comment

Uh oh!

develric May 21, 2021

Choose a reason for hiding this comment

Uh oh!

mzorz May 14, 2021

Choose a reason for hiding this comment

Uh oh!

develric May 21, 2021

Choose a reason for hiding this comment

Uh oh!

mzorz May 14, 2021

Choose a reason for hiding this comment

Uh oh!

develric May 21, 2021

Choose a reason for hiding this comment

Uh oh!

mzorz May 21, 2021

Choose a reason for hiding this comment

Uh oh!

mzorz left a comment

Choose a reason for hiding this comment

Uh oh!

develric commented May 21, 2021

Uh oh!

mzorz left a comment

Choose a reason for hiding this comment

Uh oh!

peril-wordpress-mobile bot commented May 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

develric commented Mar 15, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Mar 15, 2021 •

edited

Loading

peril-wordpress-mobile bot commented Mar 15, 2021 •

edited

Loading