Page MenuHomePhabricator

Add a functionality to stop the preview process and make further edits
Open, Needs TriagePublic

Description

So in current version of VideoCutTool one cant stop preview process midway to return back to the video for further edits.I would be better from the UX point of view to have this functionality.

Event Timeline

Can we make this as feature req/enhancement
Also lets give it a priority
cc @Punith.nyk

Hi @Chimnayyyy @Punith.nyk lets have a plan for this task before junping straight into implementation.

Currently the flow is we spawn another worker thread in node js to process the video. To make sure this functionality works we should somehow fetch that worker thread's id, and kill that process with the help of the id.

Please look around for the same/similar approach and lets discuss them here (async) before we start with the implementation.

Also since this is a crucial flow, i guess it would be really helpful that we have some unit tests in place for testing this.

Please feel free to ping me for any help required.

Thanks

Hey @Reputation22

I've been working on the video processing cancellation functionality and have made some progress. Here's my approach:

Current implementation:

  • We're already storing the ffmpeg process PID in Redis using redis.set(job-pid:${videoId}, cmd.pid) in the spawnAsyn function
  • When a user cancels a job, we set a cancel-job:${videoId} flag in Redis
  • The worker checks this flag after processing and stops if it's cancelled

Let me know if this approach makes sense or if you have alternative suggestions!

Hi @Chimnayyyy

  • I'm unable to find the ffmpeg process PID in spawnAsync function.. can you please provide link/ref to it.
  • the worker whose id we require is the one defined in worker.js, which we're creating in function onListening inside /server/index.js

Here's what i was thinking for this

  • Grab the id of the worker from there, and store it in redis (maybe)
  • Now when we spawn any worker for editing the video, update the storage in redis to map worker_id <> video_id (current video editing) relation [have to maintain this, and update it when that worker picks up a new video]
  • Now when we click on any Cancel button, we send an event for the video id (from client), and with the help of that map, we fetch the worker id of the worker which is currently editing it.
  • Now we can kill the worker process from our server, and update the (worker-video id) map.

The other approach i was thinking was around this (I'm not sure of this, have to check around this).

  • Somehow we get the worker id from the worker object (maybe via some inbuilt functions by bullmq).
  • Once we have worker id, instead of going around the redis method, we can send that worker id to client via the progress message (the one we send in spawnAsyn function)
  • And now when the client says to stop the video, we just send the worker_id from client to server, and we kill that process. (Although we need to ensure that the worker's process is killed and the worker is free, not that we lose the worker itself).

lmk what are your thoughts on the same.

hey @Chimnayyyy removing you as the assignee here, as no updates from your end. For new contributors feel free to refer to this comment for any help in implementation

Hi @Chimnayyyy

  • I'm unable to find the ffmpeg process PID in spawnAsync function.. can you please provide link/ref to it.
  • the worker which id we require is the one defined in worker.js, which we're creating in function onListening inside /server/index.js

Here's what i was thinking for this

  • Grab the id of the worker from there, and store it in redis (maybe)
  • Now when we spawn any worker for editing the video, update the storage in redis to map worker_id <> video_id (current video editing) relation [have to maintain this, and update it when that worker picks up a new video]
  • Now when we click on any Cancel button, we send an event for the video id (from client), and with the help of that map, we fetch the worker id of the worker which is currently editing it.
  • Now we can kill the worker process from our server, and update the (worker-video id) map.

The other approach i was thinking was around this (I'm not sure of this, have to check around this).

  • Somehow we get the worker id from the worker object (maybe via some inbuilt functions by bullmq).
  • Once we have worker id, instead of going around the redis method, we can send that worker id to client via the progress message (the one we send in spawnAsyn function)
  • And now when the client says to stop the video, we just send the worker_id from client to server, and we kill that process. (Although we need to ensure that the worker's process is killed and the worker is free, not that we lose the worker itself).

lmk what are your thoughts on the same.