Video compression: managing npe and tracking it in MEDIA_VIDEO_CANT_OPTIMIZE event#14114
Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
WordPress/src/main/java/org/wordpress/android/ui/uploads/VideoOptimizer.java
Outdated
Show resolved
Hide resolved
|
You can test the changes on this Pull Request by downloading the APK here. |
renanferrari
left a comment
There was a problem hiding this comment.
Hey @develric 👋
First of all, thanks for the changes! Code is looking good and I can confirm the expected behavior is happening: the NPE occurs but it is now correctly caught and the MEDIA_VIDEO_CANT_OPTIMIZE event is being fired with the correct parameter.
LGTM ![]()
hello, what do you mean of "the mp4 video linked in p2y3YZ-4nh-p2". can want to get the video to reproduce the bug. |
|
Hey @Geoffrey1014 👋 , thanks for your interest with this 🙇 !
the |
Fixes #13692
This PR introduces the management of NPE when attempting to compress a video file before the upload and align the behavior in this scenario to what we already do with other cases when getting an error during compression operations (that is fallback to uploading the video uncompressed).
An npe event is tracked in the
MEDIA_VIDEO_CANT_OPTIMIZEevent adding thewas_npe_detected=trueproperty.NOTE: we have an open PR contributed here that actually fixes the original issue but waiting for an eventual follow up this fix seems good enough also given it seems a pretty edge case looking at numbers in sentry with
m4mstring. Reporting here also this internal reference (p2y3YZ-4nh-p2) where we considered other possible approaches.To test
Not trivial to replicate since it happens only with specific videos, but you can use the mp4 video linked in p2y3YZ-4nh-p2
VideoOptimizer >) check you get a npe but it is managed (so no crash) and the video is uploaded uncompressedMEDIA_VIDEO_CANT_OPTIMIZEevent is fired and containswas_npe_detected=truePR submission checklist:
RELEASE-NOTES.txtif necessary.