lookit / ember-lookit-frameplayer

Ember app with experiment frames and a frame player to run browser-based experiments
MIT License
5 stars 19 forks source link

Investigate very long recordings #373

Closed becky-gilbert closed 3 weeks ago

becky-gilbert commented 2 months ago

Summary

We've had a few instances of very long recordings. These should have been prevented by the experiment runner via the maxRecordingTime value.

To do

If there is a bug:

ianchandlercampbell commented 1 month ago

@becky-gilbert can you provide an update on this issue?

becky-gilbert commented 1 month ago

@ianchandlercampbell

Update

This long recording file was due to the large recording time limit that was in place for the EFP version being used for that study. It was not caused by a bug in the logic around the recording time limit.

I assumed that I had inadvertently increased the maxRecordingTime value during RecordRTC development, but it turns out that session recording has had the very large default time limit since at least 2020 (this is as far back as I went into the commits). The documentation says that the default time limit is 7200 seconds, but up until recently (when we fixed this issue) that has only been the case for single trial recording.

The session recording file (app/mixins/session-record.js) from Dec 2, 2020 in this commit shows the large value in line 143 (you'll need to expand the file lines to see it):

const maxRecordingLength = 100000000;

And the video recorder service file (app/services/video-recorder.js) from Jun 2, 2020 in this commit shows the same large default value on line 147 (again, you have to expand the lines to see it):

install(videoFilename = '', pipeKey = '', pipeEnv = 1, maxRecordingTime = 100000000, autosave = 1, audioOnly = 0) {

We fixed this issue here. Since this fix, all recordings have a default maximum time limit of 7200 seconds (2 hours). This is now also the upper limit, rather than just the default. This means that any experiments using EFP versions that are older than this commit (i.e. more than 2 weeks old) have the potential to produce very long session recording files.

My suggestion is that we implement a file size check on the lookit-api side to handle the case where another very large session recording is produced. That issue is here: https://github.com/lookit/lookit-api/issues/1404

ianchandlercampbell commented 1 month ago

Just echoing that I think this is a wise solution to the problem! Thanks for investigating further. CHS gets a little more robust every week thanks to y'alls continued efforts!

becky-gilbert commented 3 weeks ago

Closed via #377