Replace existing video recorder
Closed this issue ยท 3 comments
Summary
Currently, EFP uses Pipe to record participants. The expenses for the Pipe have risen, and with their non-response to security reviews, we've decided to replace Pipe with our own implementation. This change will heavily lean on the RecordRTC library.
Implementation
Create a class that implements the record
, stop
, getTime
, install
and destroy
methods from app/services/video-recorder.js
. A few of these methods might not be needed once we get into the RecordRTC API. Additionally, the hook design pattern from Pipe in addition to the promise pattern is a bit murky to wade through. If possible, we should do either one and not both.
When video is recorded, the file should be downloaded to client. This will be a placeholder until we get to the upload to amazon issue completed (Issue number to come).
Generally, we should change as little as possible with this API to save time on implementing. This Feature will require quite a bit of QC and we should spend our time there.
AC
- Implement methods
- Implement Hooks
Much of this is now implemented on the replace-pipe
branch, though it needs cleaning up and maybe refactoring.
Here are some issues I'm still working on:
- The mic check is being called every time the recorder starts up. I think it should only run at the request of the frame (and I think the only frame that uses it is
exp-video-config
). - Timestamps aren't working yet. We need to get accurate timestamps from the recording stream and use them in
getTime
. - There's a bug in the video recording that's specific to the
exp-lookit-video
frame, which causes the video recording blob to be empty at the end of the frame (but recording does work throughout the frame up to that point).
And here are the frames that I've tested so far:
exp-video-config
exp-lookit-video-consent
exp-lookit-images-audio
exp-lookit-video
I'll edit the lists above as I get more done โ
@mekline @okaycj Here are some long-overdue updates on the status of the replace-pipe
branch.
โ = done, ๐ง = in progress/to do
โ
- Mic check: this no longer runs every time the recorder starts up. Now it only runs at the request of the frame (when the private checkMic
variable is true
), and the only frame that uses it is exp-video-config
. The mic check continuously processes the audio input stream and will continue until a volume threshold is reached, so for browser performance reasons I think it should be limited to when it's actually needed/used.
Note: I'm not sure how this currently works because the mic check is handled by Pipe behind the scenes, but I can look into if we think it's important?
โ
- Timestamps: fixed.
โ
- Bug related to empty data in the exp-lookit-video
frame: fixed.
โ
- AWS S3
class: integrated with VideoRecorder
class (thanks for your help @okaycj!). The recorder's data is checked every 1 second, and if the unsaved data has reached 5 MB then it is sent as part of a multi-part upload (unless the total recorded data is <5 MB or it is the last part of the upload).
โ
- Display: fixed some webcam display/formatting differences between Pipe and RecordRTC versions, which were caused by losing some CSS that came from Pipe.
Note: There are still some minor display differences that I have not fixed/implemented. For instance, in exp-video-config
, Pipe shows a small volume meter during webcam display, and the current/total time values during recording playback.
โ
- Session recording: currently implementing and testing
๐ง - Video upload error handling: currently implementing and testing
Here's a brain-dump of things to test (either 'manually' or via Ember tests). Please comment below or edit this list if I've missed anything:
- Single-frame recording via
doRecording: true
frame parameter - check all frames -
maxRecordingLength
parameter - Session recording via
exp-lookit-start-recording
andexp-lookit-stop-recording
frames - Session recording via
startSessionRecording
andendSessionRecording
parameters supported by all frames (though the docs say that this method is not recommended - are we still supporting it?) - Very short (single frame) recordings are uploaded: <1 second
- Very long (session) recordings are uploaded: what's a reasonable duration?
- Upload takes longer than
maxUploadSeconds
(test by reducing max time and/or throttling connection?) - Upload fails (disconnect from internet during recording)
- User closes tab/window during recording
- Setting the various "wait for recording" and "wait for upload" parameters
- Set frame's
doRecording: true
during session recording (session recording should take precedence) - Basic benchmarking against Pipe recordings (video file length/size from single-frame recordings of a specified duration)
@mekline @okaycj Apologies in advance for the long post. I'm cleaning this branch up to get it ready for review, and wanted to raise two scope issues (1 and 2 below) as well as ask a question about implementing the Pipe connection hooks (3 below).
1. Scope issue
I recently came across some Pipe DOM elements that we don't get with Record RTC and thus may want to implement but that I don't think will make it into this shape. These are all things related to displaying the video recording stream back to the user, and are mainly used in the video-consent and video-config-quality frames.
- 'pipeMenu' showing record/stop/play buttons
- 'pipeTimer' showing the current recording duration in seconds
- 'pipeCounter' showing the current time and total duration during playback
- 'pipeMicContainer' showing a mic icon with audio levels
So far I'm just using the standard controls that come with HTML video elements along with a separate "record" and "stop recording" buttons, so the record/stop/play functionality is still there but the UI looks a little different. I have not implemented the timer, counter, or mic levels (although the default video element shows current/total time during playback). I'll add screenshots soon.
I won't have time to implement these UI elements before the end of the current shape, so I'd like to suggest that we think about addressing them in a future shape. I'm planning to leave the CSS in place for these absent Pipe-related elements so that in the future we can add them and re-use the CSS where possible.
2. Scope issue
There are a few Pipe hooks listed in the code comments that don't appear to be used by any frames. Some of these are also related to the Pipe menu that I haven't fully replaced. So I'm going to suggest that we leave them out of this shape as I don't think I'll have time to implement them and still get the testing done. This does mean that the upcoming Record RTC EFP release could break a researcher's custom frames that rely on any of these hooks, but hopefully this isn't an immediate problem since the Pipe EPF versions will still be available.
Here are the specific Pipe hooks that I haven't implemented yet and that don't seem to be used in our code. There are more detailed descriptions of each of these in the Pipe documentation.
btRecordPressed
: user presses the Pipe menu record button.btPausePressed
: user presses the Pipe menu pause recording button.onPlaybackComplete
: after the user replays their video recording via the Pipe menu.onSaveOk
: when the video data has been sent back to the Pipe server. This is very similar to the currently implemented/usedonUploadDone
hook, except that the latter fires after all uploading steps are complete (including file conversions etc.) so it fires after this one.
3. Implementation question
There are two Pipe connection-related hooks don't make as much sense now that we've switched from Pipe's continuously-connected streaming upload to discrete multi-part uploads. These are onConnectionClosed
and onConnectionStatus
. I've implemented them anyway and tried to mimic their previous behavior.
onConnectionStatus
: now triggered by the recorder'srecord
method (status = "connected" after the S3 class successfully initiates the multi-part upload) and theonConnectionClosed
hook (status = "disconnected" whenever the connection closes)onConnectionClosed
: now triggered by theonUploadDone
hook (when the S3 class successfully completes the multi-part upload) and the recorder'sdestroy
method (connection should be considered closed when this recorder doesn't exist)
I'm interested in whether you have any feedback on how I've implemented these hooks and suggestions for how to make them function more similarly to the Pipe version.