Extending video recording extension

[becky-gilbert]

Hi @jspsych/core, we're hoping to extend the recording extension for our project (CHS/Lookit) video recording extension, and use our extended version to push the recorded chunks into our own storage. However, the problem we're having when we try to extend the RecordVideoExtension class is that everything is either marked as private or not accessible because it is instantiated as a property rather than a method. E.g.

class RecordVideoExtension implements JsPsychExtension {
  static info: JsPsychExtensionInfo = {
    name: "record-video",
  };

 // ...

 // we can't access recordedChunks etc. because they're private

  private recordedChunks = [];

  // we can't override these properties (initialize, on_start etc.) because typescript throws an error

  initialize = async () => {};
  
  on_start = (): void => {
    // ...
  }

Questions:

1. Can we mark the properties such as recordedChunks as public?
2. Can we rewrite the property functions (on_start etc.) as methods?

Please let us know if we're missing something obvious here! Do you know of any other users who have added features to extension classes? We'd be happy to reference their solutions.

We can do what we need to do without extending this class, so if these changes aren't possible, we can create our own recording extension rather than extending the existing one. We just thought it would be nice to extend the RecordVideoExtension class to avoid re-inventing the wheel, and so that we can push other features into jsPsych's class for other users (if you're interested).

[jodeleeuw]

I'm very open to modifying how Extensions are configured!

I'm not sure inheriting the class is the right solution for maintainability. We've never really thought of plugins or extensions as inheriting and modifying other plugins or extensions. My concern is that it creates a set of dependencies that we didn't configure the project to maintain. @bjoluc has good instincts on this kind of thing, so hoping he will chime in. I'm happy to be convinced that I'm wrong by either of you!

Is there a way that the RecordVideoExtension could be modified directly to support the use case? Like adding a method that sets how to handle the updateData() event? Or would that still be too clunky on the CHS side?

[becky-gilbert]

Thanks for your input Josh! I will wait to hear more from @bjoluc and @okaycj.

Is there a way that the RecordVideoExtension could be modified directly to support the use case? Like adding a method that sets how to handle the updateData() event? Or would that still be too clunky on the CHS side?

Yes the extension would need to be modified to take a user-defined data handler function (probably a good idea for all jsPsych users anyway!). We were also going to add some default behavior and optional parameters to our CHS version - things like controlling what happens while the recording is first starting up and while the upload finishes (error handling, time limits, optional content to show to the participant). I don't think this is possible with the current recording extension. So we would probably also need on_start and on_finish hooks. Or we could add this functionality to the extension code, along with some optional parameters.

[jodeleeuw]

If we added all the hooks that are necessary would that solve the problem, or would it still create a difficult-to-manage situation because you have to inject the code for those hooks into the user's experiments?

[bjoluc]

I'm not sure inheriting the class is the right solution for maintainability. We've never really thought of plugins or extensions as inheriting and modifying other plugins or extensions. My concern is that it creates a set of dependencies that we didn't configure the project to maintain.

Good point. I don't think this is an issue though: Extension instances are publicly exposed via the jsPsych.extensions object and calling (public) extension methods from user code is an officially documented pattern (e.g. webgazer extension). Since public methods of an extension are part of the extension's API, breaking changes in these are limited to major releases. As long as we stick to this rule, it's perfectly fine to build extensions as sub classes of other extensions.

Can we mark the properties such as recordedChunks as public?

I don't think that's a good idea as it would publicly expose the internal implementation. But adding additional overridable methods along the lines of on_data_update like suggested by @jodeleeuw sounds like a good approach.

Can we rewrite the property functions (on_start etc.) as methods?

Definitely, yes. As long as you either call autoBind(this); in the constructor or add a this.my_function.bind(this) for each method, there's no functional difference to arrow functions.

[becky-gilbert]

Thanks very much for your quick feedback @jodeleeuw and @bjoluc! I'm going to wait to chat with @okaycj about this and get back to you.

[okaycj]

Is there a way that the RecordVideoExtension could be modified directly to support the use case? Like adding a method that sets how to handle the updateData() event? Or would that still be too clunky on the CHS side?

and

I don't think that's a good idea as it would publicly expose the internal implementation. But adding additional overridable methods along the lines of on_data_update like suggested by @jodeleeuw sounds like a good approach.

I will always be looking for a solution that doesn't require upstream changes, but adding a method such as mentioned above would be amazing and could make our lives easier. Additionally, could there be a way for the extension to pass the data through on_data_update and not store it? A boolean option would be fine.

Thank you @bjoluc for the quick bind lesson and @jodeleeuw for the suggestion. Please let me know if anyone needs anything.

[jodeleeuw]

Additionally, could there be a way for the extension to pass the data through on_data_update and not store it? A boolean option would be fine.

That seems very reasonable to me.

Happy to make upstream changes here given that they will be improving the functionality for everyone!

[becky-gilbert]

Thanks @jodeleeuw! I can do a quick first pass at this, if that would be helpful?

[jodeleeuw]

Yes please! Thanks @becky-gilbert