search

nextcloud / talk-ios

📱😀 Video & audio calls through Nextcloud on iOS
GNU General Public License v3.0
151 stars 91 forks source link

Support videos in media view #1844

Open DanielStandfest opened 1 month ago

DanielStandfest commented 1 month ago

added support for videos (and playback) when swiping in media view.

Closes #1702

@SystemKeeper

SystemKeeper commented 1 month ago

Very cool @DanielStandfest, thanks a lot for working on this 👍 We are going to take a look at this in the upcoming week.

Btw, we can add you to the Nextcloud org if you like? Then you can directly push here. We also have a developer chat where we can create a guest account for you, if you like. Just let me know :)

DanielStandfest commented 1 month ago

Thank you @SystemKeeper. Sure, would be nice to be added :)

Appreciate it.

SystemKeeper commented 1 month ago

The warning about the branch being out of date is usually not an issue, it's just a warning. If merging would not be possible you would see a message like:

image

Since we pull in latest translation changes at least once a day, it's very likely that your branch becomes outdated quite quickly.

If you wanna update the branch to be inline with latest master, you can use the rebase option to prevent unnecessary merge commits btw:

image
github-actions[bot] commented 3 weeks ago

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

DanielStandfest commented 2 weeks ago

Any news on this one?

@SystemKeeper

Ivansss commented 1 week ago

Hello @DanielStandfest ,

Thanks a lot for your PR :)

Sorry for our late reply, @SystemKeeper and I did take a look to your PR last week but forgot to write a reply here.

We noticed a few things that could be addressed before merging your PR:

  1. It would be nice if the NCMediaViewerViewController is opened directly when tapping on a video file too: https://github.com/nextcloud/talk-ios/blob/ba727cd6e011251d9d8bbeaede1182a1cb6a855d/NextcloudTalk/BaseChatViewController.swift#L3312-L3319

  2. We could skip for now "webm" and "mkv" videos and do not try to present them in the NCMediaViewerViewController and just present them using VLC: https://github.com/nextcloud/talk-ios/blob/ba727cd6e011251d9d8bbeaede1182a1cb6a855d/NextcloudTalk/BaseChatViewController.swift#L3536

  3. (Optional for this PR, could be done in a follow-up PR by us or you) Instead of directly downloading videos when presenting them in the NCMediaViewerViewController, we could show a play button and the preview of the video (if available) and only when tapping on the play button we start downloading the video

DanielStandfest commented 1 week ago

Hi @Ivansss,

thank you for your valuable feedback!

Points 1 and 2 have now been implemented. Additionally, I noticed the issue with unsupported video formats when swiping through webm and mkv videos. I’ve adjusted the getPreviousFileMessage and getNextFileMessage methods to address this.

Regarding point 3, I’d suggest handling it as a separate feature in a future update, and I recommend creating a dedicated issue to track it.

Thanks again for your insights!