MM-54866 - Calls: Transcription support #7703

cpoile · 2023-12-07T20:13:45Z

Summary

Adds subtitles support for calls recordings.
~~NOTE: waiting on a UI element (the "cc" icon) to be added to compass-icons, that's why it's marked Do Not Merge. Otherwise, it's ready for review.~~ Updated, can merge now. New screenshots in the thread.
Includes a patch to the react-native-video package which fixes how it downloads subtitles. Previously, it was using a deprecated method which swallowed errors. The patched version uses the newer callback method, logs any errors, and uses a dispatch group (essentially a wait group) to sync before loading the video with the subtitles (if any). This doesn't block the main UI thread (the other methods I tried do block).

UX Note: the figma has the color of icons as greyish, but the icons currently are #fff, so I kept that (I didn't want to change all the icons in this PRs).

iOS captions enabled	iOS captions disabled

Android captions enabled	Android captions disabled

Android tabled captions enabled	Android table captions disabled

Ticket Link

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
~~Have tested against the 5 core themes to ensure consistency between them.~~ na

Device Information

Android: 13, Pixel 6
Android: 13, Galaxy Tab s7+
iOS: 16.5.1, iPhone 14

Screenshots

Release Note

Calls: Added transcription subtitles support to Calls recordings.

cpoile · 2023-12-07T22:57:12Z

@matthewbirtch Updated the compass icon, thanks. Here are a couple new screenshots:

iOS cc on	iOS cc off

patches/react-native-video+5.2.1.patch

app/screens/gallery/video_renderer/index.tsx

matthewbirtch · 2023-12-08T14:03:16Z

@matthewbirtch Updated the compass icon, thanks. Here are a couple new screenshots:

Thanks Chris, the updated icon looks good.

For the actual captions, do we have control over the style, size and position of the text? I had hoped we could do something like the below screenshots. From the screenshots you've shared above, the captions look like they will be hard to read. Figma

What's possible here? Does the caption need to live within the video frame?

cpoile · 2023-12-08T16:11:43Z

@matthewbirtch Unfortunately we don't have control over the subtitles -- styling is only supported on android, and then only in the newest version of the video package (which is beta, so we shouldn't upgrade until it stabilizes). I'm pretty sure there's a way if we do our own native code for it, but that would take some time.

mattermost-build · 2023-12-08T16:45:07Z

Building app in separate branch.

matthewbirtch

Ok, so aside from the styling and position (which we can't do anything with), a few things I noticed:

There may be a collision with the button that appears next to the CC button. When tapping the CC button, at times it seems to be actually be executing a download (since the download button is right next). Is it possible the download button has its hit area overlapping with the CC button somehow?
It seems that apostrophe's aren't displaying properly in the captions (though they seem fine in the transcription file)
On iOS, when I turn off the captions, it doesn't hide the captions immediately. It seems to wait until the next line of text to hide.
On Android in landscape mode, it looks like the subtitles are getting cut off at the bottom
For landscape mode in general (iOS and Android), is there any way to move the subtitles up a bit - the video player controls obscure it while they are showing.

cpoile · 2023-12-08T19:09:22Z

@matthewbirtch Thanks for the feedback:

Fixed -- I had changed the button container sizes to be in line with the designs, but didn't reduce the hit areas. Now they are the size of the button + 4 px, which just touches the next button b/c of the 8px gap.
That's a known issue that we'll fix server-side.
That's an issue with the iOS player, our changes don't affect the subtitles until the next one is rendered.
and 5. This is a consequence of not having control over the subtitle styling that I described earlier. We'll have to either accept it, or I could backport the subtitle styling for Android (with some effort). If we did that, iOS would still be the same, unless we spent even more time on that (unknown amount).

mattermost-build · 2023-12-08T21:24:23Z

Building app in separate branch.

matthewbirtch

Thanks Chris. Looks like you've fixed the items you're able to. Not much we can do about the others by the sounds of it.

streamer45

Looks great, left some minor comments.

app/products/calls/utils.ts

app/screens/gallery/index.tsx

patches/react-native-video+5.2.1.patch

streamer45 · 2023-12-21T20:53:05Z

@cpoile Could you confirm whether the attached text transcript is previewable from Android? I had issues showing it but maybe it's my local setup at fault.

cpoile · 2023-12-21T20:54:07Z

@streamer45 oh yah, interesting, there's no inline view of the transcript text file. :/

streamer45

Approving, if we can fix the text preview great, otherwise let's file a ticket. Thanks!

cpoile · 2023-12-21T21:20:11Z

Doesn't look like a quick fix, so filed: https://mattermost.atlassian.net/browse/MM-56365

* captions on videos from posts and searches * add the patch for react-native-video which fixes subtitle downloading * improve spacing * fix patch file * upgrade compass-icons; use cc icon * revert patch overwrite * fix patch * use useMemo * fix hitslops on pressables * use new Caption format; refactor for clarity * simplify tracks creation and use

cwarnermm · 2024-01-24T18:58:33Z

@cpoile - Are you open to helping draft the product documentation for this functionality?

* captions on videos from posts and searches * add the patch for react-native-video which fixes subtitle downloading * improve spacing * fix patch file * upgrade compass-icons; use cc icon * revert patch overwrite * fix patch * use useMemo * fix hitslops on pressables * use new Caption format; refactor for clarity * simplify tracks creation and use

cpoile added 3 commits December 7, 2023 14:24

captions on videos from posts and searches

bb4d8c8

add the patch for react-native-video which fixes subtitle downloading

f2cffff

improve spacing

d1318d1

cpoile added 2: Dev Review Requires review by a core commiter Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) 2: UX Review Requires review by a UX Designer labels Dec 7, 2023

cpoile requested review from streamer45, matthewbirtch and enahum December 7, 2023 20:13

mm-cloud-bot added release-note labels Dec 7, 2023

cpoile added 2 commits December 7, 2023 16:04

fix patch file

846cca7

upgrade compass-icons; use cc icon

20c6da4

cpoile removed the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Dec 7, 2023

enahum requested changes Dec 8, 2023

View reviewed changes

patches/react-native-video+5.2.1.patch Show resolved Hide resolved

app/screens/gallery/video_renderer/index.tsx Outdated Show resolved Hide resolved

cpoile added 3 commits December 8, 2023 09:35

revert patch overwrite

ec458a6

fix patch

d1d25aa

use useMemo

5be8100

cpoile requested a review from enahum December 8, 2023 16:12

matthewbirtch added the Build Apps for PR Build the mobile app for iOS and Android to test label Dec 8, 2023

mattermost-build removed the Build Apps for PR Build the mobile app for iOS and Android to test label Dec 8, 2023

matthewbirtch requested changes Dec 8, 2023

View reviewed changes

fix hitslops on pressables

5adfce7

matthewbirtch added the Build Apps for PR Build the mobile app for iOS and Android to test label Dec 8, 2023

mattermost-build removed the Build Apps for PR Build the mobile app for iOS and Android to test label Dec 8, 2023

enahum approved these changes Dec 8, 2023

View reviewed changes

matthewbirtch approved these changes Dec 9, 2023

View reviewed changes

matthewbirtch removed the 2: UX Review Requires review by a UX Designer label Dec 11, 2023

use new Caption format; refactor for clarity

aaf7b5c

streamer45 reviewed Dec 19, 2023

View reviewed changes

app/products/calls/utils.ts Outdated Show resolved Hide resolved

app/products/calls/utils.ts Show resolved Hide resolved

app/screens/gallery/index.tsx Show resolved Hide resolved

patches/react-native-video+5.2.1.patch Show resolved Hide resolved

simplify tracks creation and use

5f399d5

cpoile requested a review from streamer45 December 21, 2023 20:48

streamer45 approved these changes Dec 21, 2023

View reviewed changes

streamer45 added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Dec 21, 2023

cpoile merged commit 2c1f318 into main Dec 21, 2023
7 checks passed

cpoile deleted the MM-54867-transcription-support branch December 21, 2023 21:20

amyblais added this to the v2.13.0 milestone Jan 2, 2024

amyblais added the Docs/Needed Requires documentation label Jan 2, 2024

cwarnermm mentioned this pull request Feb 9, 2024

Added transcription end user docs mattermost/docs#6910

Merged

amyblais added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Feb 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MM-54866 - Calls: Transcription support #7703

MM-54866 - Calls: Transcription support #7703

cpoile commented Dec 7, 2023 •

edited

Loading

cpoile commented Dec 7, 2023

matthewbirtch commented Dec 8, 2023 •

edited

Loading

cpoile commented Dec 8, 2023

mattermost-build commented Dec 8, 2023

matthewbirtch left a comment

cpoile commented Dec 8, 2023

mattermost-build commented Dec 8, 2023

matthewbirtch left a comment

streamer45 left a comment

streamer45 commented Dec 21, 2023

cpoile commented Dec 21, 2023

streamer45 left a comment

cpoile commented Dec 21, 2023

cwarnermm commented Jan 24, 2024

MM-54866 - Calls: Transcription support #7703

MM-54866 - Calls: Transcription support #7703

Conversation

cpoile commented Dec 7, 2023 • edited Loading

Summary

Ticket Link

Checklist

Device Information

Screenshots

Release Note

cpoile commented Dec 7, 2023

matthewbirtch commented Dec 8, 2023 • edited Loading

cpoile commented Dec 8, 2023

mattermost-build commented Dec 8, 2023

matthewbirtch left a comment

Choose a reason for hiding this comment

cpoile commented Dec 8, 2023

mattermost-build commented Dec 8, 2023

matthewbirtch left a comment

Choose a reason for hiding this comment

streamer45 left a comment

Choose a reason for hiding this comment

streamer45 commented Dec 21, 2023

cpoile commented Dec 21, 2023

streamer45 left a comment

Choose a reason for hiding this comment

cpoile commented Dec 21, 2023

cwarnermm commented Jan 24, 2024

cpoile commented Dec 7, 2023 •

edited

Loading

matthewbirtch commented Dec 8, 2023 •

edited

Loading