search

DmcSDK / cordova-plugin-mediaPicker

cordova android ios mediaPicker support selection of multiple image and video gif ✨ cordova android 和 ios 图片视频选择cordova插件，支持多图 视频 gif，ui类似微信
MIT License
139 stars 93 forks source link

Refactor code to be more robust in multi thread environment #54

Closed guoyongxin closed 2 years ago

guoyongxin commented 5 years ago

Hi DMC,

This PR is for refine some code for multithreading environment. The current implementation for iOS is error prone. If javascript call two plugin call at same time, the callbackId will be overwritten.

And I also put several file operation into background thread, to avoid main thread responding issue.

guoyongxin commented 5 years ago

请问对这个PR有什么顾虑吗？各方面的都可以，我们可以讨论。 Thanks

guoyongxin commented 5 years ago

@DmcSDK Hi, just a kind reminder for this PR. Do you have any comments?

DmcSDK commented 5 years ago

@guoyongxin 首先非常非常感谢你的代码

在你第一次提交的时候，我当时是有点疑惑和顾虑，所以没有合并它，工作的事情就让我忘了这事，本想完成工作后仔细研究，我忘记了非常抱歉！

模拟下场景：大致场景是这样吗？比如前端执行选择照片插件时，同时也代码触发了播放音乐的插件,这时候mediapicker插件里面的callbackid就会不正确的指向播放音乐的插件的callbackID？

我的疑虑是这样的我的感觉是，似乎在用相对复杂的代码去规避极少数出现的情况，我感觉没必要，是否有更加简单的方式，或者我理解有问题我没有get到您的点。

DmcSDK commented 5 years ago

@guoyongxin 或者您方便的话可以加我的QQ3451927565讨论一下。

guoyongxin commented 5 years ago

你说的很对。目前的代码，不会出错，因为所有的Plugin Call都是在MainThread执行。包括Compress。不会有我说的多线程问题。

可能我说的不是大问题。但是我一般的习惯是，把跟IO有关的调研用 尽量放在commandDelegate runInBackground方法执行。这样网页可能流畅一些。

比如我测试拍一张全景图，30MB。项目里的前端代码。我会得到下面的Log。里面两个PluginCall是ExtractThumbnail和Compress。分别耗时378，525ms。 2018-12-14 23:14:42.913032+0800 HelloCordova[48480:9895434] THREAD WARNING: ['MediaPicker'] took '378.163330' ms. Plugin should use a background thread. 2018-12-14 23:14:43.439343+0800 HelloCordova[48480:9895434] THREAD WARNING: ['MediaPicker'] took '525.699219' ms. Plugin should use a background thread. 2018-12-14 23:14:43.538669+0800 HelloCordova[48480:9895434] Got memory pressure notification (critical) 2018-12-14 23:14:43.638168+0800 HelloCordova[48480:9895434] 0 2018-12-14 23:14:43.638234+0800 HelloCordova[48480:9895434] {"index":0,"mediaType":"image","size":5988771,"path":"/private/var/mobile/Containers/Data/Application/xxxxxx/tmp/dmcPicker/dmcMediaPickerCompress1544800483317.jpg","quality":50,"name":"dmcMediaPickerCompress1544800483317.jpg","uri":"file:///private/var/mobile/Containers/Data/Application/xxxxxxxx/tmp/dmcPicker/dmcMediaPickerCompress1544800483317.jpg"} 2018-12-14 23:14:43.638266+0800 HelloCordova[48480:9895434] 0 2018-12-14 23:14:54.290940+0800 HelloCordova[48480:9895434] System is no longer under (critical) memory pressure.

所以，如果在意这个Log的话，我们可以把比如Compress这样的IO操作放到runInBackground这样的地方执行（虽然我觉得Cordova的这个runInBackground很弱，早该用NSOperation重写了）。如果这样的话，callbackId就不可以是MediaPicker的成员变量了。因为会被不停的覆盖。比如一个for循环调用compress，那么只有最后一个compress的callback执行。

这个PR主要是想解决这个问题。我觉得优先级也不算高吧。毕竟一般也不会出blocking的问题。有些代码可能因为formatter的原因，改了格式。

我水平也不高。这里抛砖引玉。就是希望能讨论一下。看看有没有更好的做法。

DmcSDK commented 5 years ago

@guoyongxin 你好我最近想改进两个需求，一个是压缩，目前图片压缩只支持质量压缩，现在似乎很多人喜欢压缩图片到固定的宽高像素，还有小于固定的体积，你有什么好的第三方库推荐吗？ 我想把压缩独立开来做一个新插件。

还有一个需求是ios选择文件后需要把文件转移到沙盒中，在沙盒里才能操作上传文件。 所以我现在IOS的插件流程是选择界面选了图片视频点击完成就会触发复制到沙盒操作，当复制到完成后，再返回给前端一个沙盒可操作的路径，但是遇到一个问题，当视频比较大时候复制到到沙盒实在太慢了，半天才返回给前端路径，所以我有个初步想法，如果遇到视频就先返回路径，然后再需要上传时候前端手动调用复制到沙盒的方法，但从编程的角度看似乎又体验不好。

你对IOS比我熟悉，所以我想咨询下你的意见，你有什么好的意见和建议。

guoyongxin commented 5 years ago

@DmcSDK 你好，刚刚看到你的回复。我也不是什么iOS专家。粗浅说一下我的想法。

图片压缩。 我也没有用过第三方的工具，也都是用UIImageJPEGRepresentation粗略的转换。 贴几个我觉得可能有用的链接。另外接口设计上，我认为压缩比率或者宽高应该是客户端传入比较合理，最多给一个默认的压缩比率，一般不会默认修改图片的宽高。 https://github.com/976431yang/YQImageCompressor https://github.com/hucool/WXImageCompress https://www.jianshu.com/p/ae7b3dc93686 https://github.com/apache/cordova-plugin-camera/blob/master/src/ios/CDVCamera.m

视频文件过大。 你说的究竟是多大的？我能想到的就是直接压缩了。拷贝我认为是绕不过的。默认使用中等质量的视频。 https://www.jianshu.com/p/6c27edb31168 Plugin接口上，返回PluginResult使用Progress的形式，当拷贝完成时候再结束callback。比如progress时候，不返回文件地址，只返回缩略图的base64或者缩略图沙河地址。copy结束后，出发最后一次callback，里边有成功标记，和文件沙盒地址。

最近我有点忙。不然我可以上手做做看～

DmcSDK commented 5 years ago

@guoyongxin 谢谢你的思路