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

Accept Multiple video Resolutions and Canvas Recording #427

Closed Forchapeatl closed 2 years ago

Forchapeatl commented 2 years ago

Accept Multiple video Resolutions and Canvas Recording

https://user-images.githubusercontent.com/24577149/175521082-5565a39b-3180-4321-8c5a-5a811797cc1a.mp4

Hello @jywarren @TildaDares @cesswairimu and @stephaniequintana I have

Questions

  1. The flow to update web resolution is CASE A a. Turn on Camera. b. Select resolution. or
    CASE B a. Select resolution,. b. Turn on Camera. The problem is on case B our presets modal will show after every resolution is selected. I propose keep the multiple resolution button disabled until our camera is turned on.

Optimization

The frames move much slower at higher resolutions.The resolution limit is subject to to the RAM of the computer. I will have to set a maximum resolution limit for each device. reference

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

branch and has no merge conflicts 📁

gitpod-io[bot] commented 2 years ago

cesswairimu commented 2 years ago

On resolution flow: - case A as you proposed seems smooth and I am pro that. Would like to hear thoughts from others. thanks

Forchapeatl commented 2 years ago

@cesswairimu thank you for the kind remarks. I will discuss with @stephaniequintana on the styling

stephaniequintana commented 2 years ago

Great job, @Forchapeatl!! You're really truckin' along, I love it!! Congrats on this:exclamation: :star2: :boom: :dizzy:

jywarren commented 2 years ago

@Forchapeatl this is really cool! I agree with Cess that path A sounds good - i think it's OK to force people into that flow and it doesn't seem like it will prevent people from doing anything. Better to keep it simple by design!

I am wondering how we should best organize the UI here. I have a few questions - first, can we visually isolate/contain the new controls from other controls AND from each other, to simplify the view for the user, and second, in the code itself so that when the time comes, @stephaniequintana knows exactly what new code needs to be copied into the new UI?

@stephaniequintana do you have ideas on how to do this? I was thinking maybe we could... make them appear only when you click the video button... and make the record button appear as a button in the same row (and same style) as the camera button? Could we make the webcam/video/upload buttons disappear and replace them with the new controls like resolution and record? Could the record button be red? These are just a few ideas.

Hm, are we missing a commit here, though? I don't see the extra new HTML for the buttons the way I see in https://forchapeatl.github.io/infragram/index2.html - just wondering if we missed something.

I just want to say again this is really exciting, and great work!

stephaniequintana commented 2 years ago

I also agree with case A.

@stephaniequintana do you have ideas on how to do this? I was thinking maybe we could... make them appear only when you click the video button... and make the record button appear as a button in the same row (and same style) as the camera button? Could we make the webcam/video/upload buttons disappear and replace them with the new controls like resolution and record? Could the record button be red?

Currently, the new UI has the webcam/video/upload button as a dropdown that lists the resolutions. A few thoughts:

so that when the time comes, @stephaniequintana knows exactly what new code needs to be copied into the new UI?

I'm wondering about this myself. Not necessarily in what will need to be copied, but the best approach. My unexpected travel last week put me further behind that I'm comfortable being. On that note, I am home now and well rested and will work diligently this week to catch up. I'm working on finishing connecting the functions and am hoping @Forchapeatl and I can move forward with working on the same html file.

I've connected my index2.html to all of the existing js files. I know last week we discussed adding a separate infragram2.js, but I think we should use and add to what currently works. On the approach of incorporating @Forchapeatl's additions, I will go ahead and incorporate her html/UI additions into mine and once her js/functions are merged all should work :pray: Please let me know if you have any ideas on a better approach.

Forchapeatl commented 2 years ago

Hello @stephaniequintana sorry for the delay. I have updated the commits with both the dist/infragram.js and index.ntml files. I will incorporate the newUI at my end too (under this PR). I think the resolution and record buttons should be different. Let's show the record button when Webrtc returns true. I will attach my functionality to the newUI. so that we can be in sync. Thank you.

jywarren commented 2 years ago

if the button is pressed by mistake or out of curiosity then the page may need to be reloaded in order to switch back to camera/image-upload (?) - I'll play around with both, disappearing the camera/webcam buttons and making room on the same row to the right of the video icon.

Good question. I think needing to reload the site isn't the worst, especially in the v1 interface. On the other hand, in v2, the process might work differently to avoid this, or we could imagine having an x button to close this new little pane. I trust you two to think through the flow and figure something out. You might also ask other Outreachy/GSoC fellows to try it and see what seems natural or confusing to them!

Does the resolution selection and record buttons need to be separate buttons? - Just curious. We might say "Begin recording in:" resButton1, resButton2... and have them turn red once hovered over? Just a thought...

I think this sounds promising, but might also do well with testing -- especially in mobile where there is no hover effect (touchscreen). Haha someday we'll be able to detect a hover-finger-above-screen... but not yet.