shabados / presenter

Desktop app for presenting the Shabad OS Database on projectors, TVs, and live streams
https://shabados.com
MIT License
18 stars 15 forks source link

feat: zoom closed caption API integration #640

Closed saihaj closed 3 years ago

saihaj commented 3 years ago

Summary of PR

Post current line to zoom API for closed captions.

https://user-images.githubusercontent.com/44710980/110195837-c2055e00-7e05-11eb-9ad6-946a9fe302d1.mov

Time spent on PR

Additional Resources

https://support.zoom.us/hc/en-us/articles/115002212983-Integrating-a-third-party-closed-captioning-service#

bhajneet commented 3 years ago

Good work and quickly done -- Thank you.

Based on the video:

Questions:

saihaj commented 3 years ago

Need to remove vishraams from captions.

I actually fixed this 3861b4de6ece7d66ac6b11b8fa1977360c69a210 but never re-recorded

Can you control the colors? Font size? Font family? Etc.

On zoom app I was able to change font size. Don't know how to do font family or colors. Checkout this article for Viewing closed captioning

Need to add translations/transliteration (possibly as options to be turned on / off in the app)

Yes we can add this as an option. Should I do it now or in future based on user feedback?

bhajneet commented 3 years ago

Now please. Then I’ll check it out and review. Are there any special instructions for the host of a zoom meeting that I should be aware of?

On Sat, Mar 6, 2021 at 13:41 Saihajpreet Singh notifications@github.com wrote:

Need to remove vishraams from captions.

I actually fixed this 3861b4d https://github.com/shabados/presenter/commit/3861b4de6ece7d66ac6b11b8fa1977360c69a210 but never re-recorded

Can you control the colors? Font size? Font family? Etc.

On zoom app I was able to change font size. Don't know how to do font family or colors. Checkout this article for Viewing closed captioning https://support.zoom.us/hc/en-us/articles/207279736-Closed-captioning-and-live-transcription#h_01EJW2XGTA3QNDGQBV81PQ07NY

Need to add translations/transliteration (possibly as options to be turned on / off in the app)

Yes we can add this as an option. Should I do it now or in future based on user feedback?

— You are receiving this because your review was requested.

Reply to this email directly, view it on GitHub https://github.com/shabados/presenter/pull/640#issuecomment-792015183, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLZ3BZRGC4M37PIVN3WMO3TCJZNXANCNFSM4YWK2LOA .

saihaj commented 3 years ago

Are there any special instructions for the host of a zoom meeting that I should be aware of?

Yes you need to enable closed captioning on your account. Checkout zoom docs on Enabling closed caption. I think we should also create a guide for this. I will FAI that in docs repo. We can have that guide as "Learn More" link in the settings menu.

bhajneet commented 3 years ago

image

  1. Please rename to "Zoom"
  2. Please use an input that looks like an input. MUI is trash for UI/UX imo. Should be a box of some sort, preferably with rounded corners.

Maybe rename this section for the Overlay and Zoom from Tools --> "Live Stream"

Harjot1Singh commented 3 years ago

re 1) can we name this closed captioning? It could open up being a place to add api tokens for other CC services like YouTube and Facebook

2) MUI input does support being rounded, perhaps a new setting type with this commonality can be introduced for custom text input within Shabad OS settings.

On 7 Mar 2021, at 23:28, Bhajneet S.K. notifications@github.com wrote:



Please rename to "Zoom" Please use an input that looks like an input. MUI is trash for UI/UX imo. Should be a box of some sort, preferably with rounded corners. Maybe rename this section for the Overlay and Zoom from Tools --> "Live Stream"

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub, or unsubscribe.

bhajneet commented 3 years ago

Then maybe it should be called "Captioning" or "Closed Captioning"

Imo "Live stream" is too ambiguous with "Overlay"

Here are articles for YouTube and Facebook. Both support embedded EIA 608. It doesn't seem we will tackle this for a very long time. There are no node packages that seem to deal with this besides https://www.npmjs.com/package/libcaption-node.

bhajneet commented 3 years ago

@saihaj is this ready to review? If not please mark as draft so it doesn't ping us on slack

Harjot1Singh commented 3 years ago

ERROR: Cannot read property 'zoomApiToken' of undefined I got this on startup, whenever adding a global option to options.js, you've also got to add it to settings.default.json too

bhajneet commented 3 years ago

Wat? Edit: mobile GitHub app is bad

Harjot1Singh commented 3 years ago

Update

Language features

image image

Caveat: too long a string causes truncation on zoom visuals, but you can see the entire body in the zoom closed caption transcript image image

bhajneet commented 3 years ago

Please note vishraam symbols should not be shown in Zoom

On Sun, Mar 21, 2021 at 1:36 PM Harjot Singh @.***> wrote:

Reopened #640 https://github.com/shabados/presenter/pull/640.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/shabados/presenter/pull/640#event-4486995931, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLZ3B7MLGW7DV34RW3AZMLTEYVDXANCNFSM4YWK2LOA .

bhajneet commented 3 years ago

And please make the background of the input (inside the border-radius grey line) to match the "off state" of the toggles (white). Should test multiple themes to make sure the input looks correctly.

On Sun, Mar 21, 2021 at 1:49 PM Bhajneet Kohli @.***> wrote:

Please note vishraam symbols should not be shown in Zoom

On Sun, Mar 21, 2021 at 1:36 PM Harjot Singh @.***> wrote:

Reopened #640 https://github.com/shabados/presenter/pull/640.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/shabados/presenter/pull/640#event-4486995931, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLZ3B7MLGW7DV34RW3AZMLTEYVDXANCNFSM4YWK2LOA .

Harjot1Singh commented 3 years ago

And please make the background of the input (inside the border-radius grey line) to match the "off state" of the toggles (white). Should test multiple themes to make sure the input looks correctly. On Sun, Mar 21, 2021 at 1:49 PM Bhajneet Kohli @.> wrote: Please note vishraam symbols should not be shown in Zoom On Sun, Mar 21, 2021 at 1:36 PM Harjot Singh @.> wrote: > Reopened #640 <#640>. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#640 (comment)>, or > unsubscribe > <github.com/notifications/unsubscribe-auth/ADLZ3B7MLGW7DV34RW3AZMLTEYVDXANCNFSM4YWK2LOA> > . >

image

looks a little odd imo, feels like it's constantly focused

bhajneet commented 3 years ago

Open to your suggestions

On Sun, Mar 21, 2021 at 2:20 PM Harjot Singh @.***> wrote:

And please make the background of the input (inside the border-radius grey line) to match the "off state" of the toggles (white). Should test multiple themes to make sure the input looks correctly. … <#m5061811568960006057> On Sun, Mar 21, 2021 at 1:49 PM Bhajneet Kohli @.> wrote: Please note vishraam symbols should not be shown in Zoom On Sun, Mar 21, 2021 at 1:36 PM Harjot Singh @.> wrote: > Reopened #640 https://github.com/shabados/presenter/pull/640 <#640 https://github.com/shabados/presenter/pull/640>. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#640 (comment) https://github.com/shabados/presenter/pull/640#event-4486995931>, or > unsubscribe > < github.com/notifications/unsubscribe-auth/ADLZ3B7MLGW7DV34RW3AZMLTEYVDXANCNFSM4YWK2LOA>

. >

[image: image] https://user-images.githubusercontent.com/1184274/111916280-0b92b180-8a72-11eb-99cd-35b7b5984bf8.png

looks a little odd imo

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/shabados/presenter/pull/640#issuecomment-803635930, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLZ3B5U3VBXRJ6CYUVYPE3TEY2GHANCNFSM4YWK2LOA .

Harjot1Singh commented 3 years ago

Background color approach

Unfocused

image

Focused

image

Setting off color based

Unfocused

image

Focused

image

lgtm-com[bot] commented 3 years ago

This pull request introduces 1 alert when merging a7d352cada11013a980220f08d97945b916e4df8 into 0c55158a8a72d3072b9816659f99374bd9bb04ae - view on LGTM.com

new alerts:

bhajneet commented 3 years ago

Just FYI, perhaps code is not pushed, when I'm using this PR and trying to show english translations or any other options, they don't work.

saihaj commented 3 years ago

@Harjot1Singh we can add link to documentation https://docs.shabados.com/presenter/guides/integrating-closed-captioning-in-zoom-meetings

Harjot1Singh commented 3 years ago

@bhajneet correct - all in now.

bhajneet commented 3 years ago
Harjot1Singh commented 3 years ago

Reset to defaults is covered by #644, since those are general issues

Larivaar appears to work for me? image

Harjot1Singh commented 3 years ago

Or do we want the translits to also be larivaar?

Harjot1Singh commented 3 years ago

Corrected, to make inline with presenter behaviour: image

Harjot1Singh commented 3 years ago

No indication that input has been registered (user may ask: do i simply have to paste, or do i have to click out?), if there were some kind of "saved" or checkmark displayed once new input is registered, that helps give feedback to the user since there is no "save button" to click.

It's set to blur currently. Not sure what's best to indicate this. Perhaps as you say, a greyed out checkmark that becomes filled in once saved, to the right (outside of the box)? It can persist until the settings window tab changes or is refreshed/reopened?

Harjot1Singh commented 3 years ago

Any good? https://user-images.githubusercontent.com/1184274/112915677-b81a0680-90f6-11eb-8dbb-58ea08fb2d6a.mp4

bhajneet commented 3 years ago

Any good? https://user-images.githubusercontent.com/1184274/112915677-b81a0680-90f6-11eb-8dbb-58ea08fb2d6a.mp4

Not bad, can you make the default state more see-through and use a brighter color for the "activation"? The change in color for day theme is too nuanced imo. Also I think it would be more "direct" if it were on keyup/mouseup rather than blur. That way you don't even need the default state, it would just be a checkmark that fades away after 1 or 2 seconds.

E: Also if clicking inside the text box would select all the text it would make pasting a new link faster/easier. Similar to how activating search currently selects the original query.

bhajneet commented 3 years ago

Larivaar appears to work for me?

I don't know why mine doesn't work

https://user-images.githubusercontent.com/14130567/113046419-10401f80-916e-11eb-9a52-f00ace9c6eea.mp4

E: Possibly related terminal output below

image

Harjot1Singh commented 3 years ago

Larivaar appears to work for me?

I don't know why mine doesn't work

2021-03-30.15-37-40.mp4 E: Possibly related terminal output below

image

Could you pull and try again? What node version are you on?

bhajneet commented 3 years ago

My repo was already up to date when I last posted. (Nothing to pull).

node --version && npm --version
v14.15.0
6.14.8
Harjot1Singh commented 3 years ago

My repo was already up to date when I last posted. (Nothing to pull).


node --version && npm --version

v14.15.0

6.14.8

Would you have a go at reproducing it again please? Technically, the error logs indicate that zoom shouldn't receive any captions...

bhajneet commented 3 years ago

Would you have a go at reproducing it again please? Technically, the error logs indicate that zoom shouldn't receive any captions...

I've PM'd you a longer video from beginning to end with the terminal, zoom, and shabad os windows showing. I'm having 0 issues reproducing this again and again, unless you can find something wrong with my method.

bhajneet commented 3 years ago

Larivaar in zoom is working now, thanks.

Any good? https://user-images.githubusercontent.com/1184274/112915677-b81a0680-90f6-11eb-8dbb-58ea08fb2d6a.mp4

Not bad, can you make the default state more see-through and use a brighter color for the "activation"? The change in color for day theme is too nuanced imo. Also I think it would be more "direct" if it were on keyup/mouseup rather than blur. That way you don't even need the default state, it would just be a checkmark that fades away after 1 or 2 seconds.

E: Also if clicking inside the text box would select all the text it would make pasting a new link faster/easier. Similar to how activating search currently selects the original query.

Last two things left

saihaj commented 3 years ago

Kapture 2021-04-25 at 20 50 14

bhajneet commented 3 years ago

Saihaj will you have time to checklist any remaining items in this PR?

saihaj commented 3 years ago

I think this is the only thing left. Just need to use some sort of timer as we discussed in slack I will try to push changes

use keyup/mouseup for saving input, make checkmark brighter (maybe add a green css variable to the themes?), and fadeaway check mark if no keyup/mouseup in last 3 seconds

bhajneet commented 3 years ago

Please share a gif

saihaj commented 3 years ago

@bhajneet Kapture 2021-06-10 at 22 12 19