search

publiclab / infragram

A minimal core of the Infragram.org project in JavaScript
https://infragram.org/sandbox/
GNU General Public License v2.0
44 stars 166 forks source link

Process Video Locally Infragram.org #421

Closed Forchapeatl closed 2 years ago

Forchapeatl commented 2 years ago

Process Video Locally
Fixes #418
found at https://forchapeatl.github.io/infragram/index2.html

https://user-images.githubusercontent.com/24577149/174498132-3f0b65a8-a498-40e8-8fd3-6c8f0295fa56.mp4

TEST RESULTS

Passed Test cases

Failed Test cases

Hello @jywarren @TildaDares @stephaniequintana The demo video is processed locally , but it might take a while for the video to load up the github page It seems to work only after camera has been turned on.This bug is taking a while.

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

welcome[bot] commented 2 years ago

Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help. Dangerbot will test out your code and reply in a bit with some pointers and requests. There may be some errors, but don't worry! We'll work through them with you! 👍🎉😄
One thing that can help to get started is to make sure you've included a link back to the original issue you're solving, in the format fixes #0000 (for example). And to make sure the PR title describes what you're trying to do! (often it can be the same as the issue title) Thanks! 🙌
Then, you can say hello in our chatroom & share a link to this PR to get a review! 👋 ✅

gitpod-io[bot] commented 2 years ago

jywarren commented 2 years ago

Hi! I hope to have a chance to give this a thorough read through in the next day, but I wonder if the camera start bug has to do with having to wait until it's initialized; there may be a need to listen for the right event?

jywarren commented 2 years ago

Hi @Forchapeatl - i wanted to try to help you pull apart the new code you've added to the /dist/infragram.js file, and so I made a new PR just to get a good 'diff' view -- at https://github.com/publiclab/infragram/pull/422/files?diff=unified&w=1

What I'm going to try to do is comment on each section and point to where those changes could be made in the /src/ directory. Let's try to make the changes there!

jywarren commented 2 years ago

OK - it's not too complex - there are about 6 or so sections which need to be copied into corresponding code in /src/io/camera.js and /src/ui/interface.js -- press "load diff" and you'll see my comments at https://github.com/publiclab/infragram/pull/422/files?diff=unified&w=1 !

If you make those changes in the Forchapeatl-patch-1 branch, then run grunt build, you'll generate a new version of the compiled file called dist/infragram.js. Check that file in and then remove /dist/infragram3.js, and point index3.html at /dist/infragram.js, and we should be OK!

Forchapeatl commented 2 years ago

okay @jywarren ,thank you

Forchapeatl commented 2 years ago

Hello @jywarren , @cesswairimu , @TildaDares the code base has been structured as requested on https://github.com/publiclab/infragram/pull/422/files?diff=unified&w=1

jywarren commented 2 years ago

This looks excellent @Forchapeatl -- can you run grunt build and check in the updated dist/.... files? Then I'm going to give it a quick test in GitPod and we should be able to merge -- great work!!

jywarren commented 2 years ago

No rush as it's very late here so I won't get to it until tomorrow. Congrats on the changes, they were complex!

Forchapeatl commented 2 years ago

Thank you, @jywarren

jywarren commented 2 years ago

Hi @Forchapeatl I didn't see the dist files added again, just wanted to check if you needed any help?

Forchapeatl commented 2 years ago

Hello @jywarren , all is well with grunt. This PR functionality is a bit limited / buggy . Please I will include the dist file once I am done with these limitations / bugs.

Video processing works only after camera has been on. Accommodate large video files > 10MB.

Please permit me redirect you to another PR https://github.com/publiclab/infragram/pull/427

jywarren commented 2 years ago

Video processing works only after camera has been on. Accommodate large video files > 10MB.

No problem, but I think the 10MB might be a tough one - i'm not sure how you're storing it but if it's in localStorage, there is a hard JS limit on how much can be stored. If you think it's a big issue, we could push people into the workflow of recording on their device, then uploading the recording and converting it, rather than recording live within the website. What do you think? So people would upload a video file in the same way as they would have uploaded a still image. @stephaniequintana also curious what you think about that workflow as an alternative. It could be simpler and could sidestep some of these issues?

jywarren commented 2 years ago

Oh darn i'm being silly - this is a prerecorded video, and you're just saying that there was a 10mb limit to the processing of that "upload" - but was my idea correct that it was due to a localStorage file size limitation?

jywarren commented 2 years ago

I want to say that we have processed larger images in https://github.com/publiclab/image-sequencer/ or https://github.com/jywarren/webgl-distort/ - I could be wrong, but I think we were just keeping such larger amounts of data in memory, or in the HTML DOM itself. Where is the 10mb limit happening?

Forchapeatl commented 2 years ago

Video Processing works now works without clicking the webcam button.

https://user-images.githubusercontent.com/24577149/179400204-050add7e-c809-48ed-a3a5-8a9e9fddd425.mp4

jywarren commented 2 years ago

Wow excellent! @Forchapeatl how are you thinking in terms of what should be ready to merge next? Thank you!

Forchapeatl commented 2 years ago

You 're welcome @jywarren

jywarren commented 1 year ago

Ok no worries and great to see the other pr! My internet isn't great here so I hope to review tomorrow. If you can get some feedback from another mentor or a fellow intern before then that's great too!

On Sat, Jun 25, 2022, 7:54 PM FORCHA @.***> wrote:

Hello @jywarren https://github.com/jywarren , all is well with grunt. The functionality is a bit buggy . Please I will include the dist file once I am done with these bugs.

Video processing works only after camera has been on. Accommodate large video files > 10MB.

Permit me redirect you to another PR #427 https://github.com/publiclab/infragram/pull/427

— Reply to this email directly, view it on GitHub https://github.com/publiclab/infragram/pull/421#issuecomment-1166292222, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAF6J6TKV4D7DQGFPYLAPTVQ4FRXANCNFSM5ZG7WCCA . You are receiving this because you were mentioned.Message ID: @.***>