Page MenuHomePhabricator
Create Task
Maniphest T277369

Format and refactor VideoCutTools worker and add progress bar
Closed, ResolvedPublic
Actions

Assigned To
Khr2003
Authored By
Khr2003
Mar 13 2021, 12:55 PM
Tags
Referenced Files
F34174200: image.png
Mar 20 2021, 3:40 AM
F34174204: image.png
Mar 20 2021, 3:40 AM
F34174209: image.png
Mar 20 2021, 3:40 AM
F34174197: image.png
Mar 20 2021, 3:40 AM
Subscribers
Aklapper
Gopavasanth
Khr2003

Description

Format worker code with these settings:

{
	"tabWidth": 2,
	"useTabs": true,
	"arrowParens": "avoid",
	"printWidth": 100,
	"singleQuote": true,
	"trailingComma": "none"
}

Refactor code. Some examples:

  • Make less repetitive i.e. cmd.stderr, cmd.on('error' and cmd.on('close' are repeating for video manipulation functions. They need to be added once.
  • Remove unused variables.
  • Change var to let, const as scope appropriate.
  • Rename function like 'callback' to more descriptive names.

Also, add a progress bar to display current video status with bitrate, frame, time and speed data.

Code repos:

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Format and refactor codebaselabs/tools/video-cut-tool-workermaster+2 K -564
Added endpoints for progress updatelabs/tools/video-cut-tool-back-endmaster+16 -1
Added progress bar and refactored workers codebaselabs/tools/VideoCutToolmaster+143 -182
Customize query in gerrit

Related Objects
Search...

Event Timeline

Khr2003 created this task.Mar 13 2021, 12:55 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 13 2021, 12:55 PM
gerritbot added a comment.Mar 14 2021, 10:18 AM

Change 671933 had a related patch set uploaded (by Khr2003; owner: Khr2003):
[labs/tools/video-cut-tool-worker@master] Format and refactor codebase - Changed code to native promise function - Unified reptitive code into single one - Combined audio disable, rotation and cropping into one command - Added prettier and eslint configuration files

https://gerrit.wikimedia.org/r/671933

gerritbot added a project: Patch-For-Review.Mar 14 2021, 10:18 AM
Khr2003 renamed this task from Format and refactor VideoCutTools worker to Format and refactor VideoCutTools worker and add progress bar.Mar 19 2021, 1:36 PM
Khr2003 updated the task description. (Show Details)
gerritbot added a comment.Mar 19 2021, 1:43 PM

Change 673486 had a related patch set uploaded (by Khr2003; owner: Khr2003):
[labs/tools/VideoCutTool@master] Format and refactor codebase - Changed code to native promise function - Unified reptitive code into single one - Combined audio disable, rotation and cropping into one command - Added prettier and eslint configuration files - Added readme file inside videos folder to add folder to git - Added progress bar to show video process

https://gerrit.wikimedia.org/r/673486

gerritbot added a comment.Mar 19 2021, 1:50 PM

Change 673487 had a related patch set uploaded (by Khr2003; owner: Khr2003):
[labs/tools/video-cut-tool-back-end@master] Added endpoints for progress update

https://gerrit.wikimedia.org/r/673487

Gopavasanth added a parent task: T264878: Progress bar while uploading video to tool and then to commons.Mar 19 2021, 7:08 PM
Gopavasanth subscribed.Mar 20 2021, 3:40 AM

Hi @Khr2003! So sweet work! Kudo's \o/!! After trying out locally, I got some feedback to you:

  • Tool's font got affected, see these two images, not just in the header but in other places too, Maybe due to
BeforeAfter
image.png (64×1 px, 11 KB)
image.png (64×1 px, 12 KB)
  • Great progress bar, what does the timing 00:00:16:86 mean? It's not like HH:MM:SS:MS right? Possible to convert into HH:MM:SS:MS formate?

image.png (190×817 px, 63 KB)

  • When the user selects Trim and other settings (Crop, Rotate, Disable Audio). It's better to perform Trip first as the video length will be decreased so it's quicker to apply other options on the resultant video.
  • Also wondering if we can use a single progress bar for all the operations (Trim, Rotate...). Currently, it seems like different progress bars for trim and the rest of the functionalities as it's two different commands I understand.
  • I edited this video locally and the resultant video is too long on the screen as shown in the below image, I suspect because I selected Rotate video too
    image.png (1×918 px, 284 KB)
  • Also wondering if you could document/have comments in the codebase as these are breaking changes.
Khr2003 added a comment.EditedMar 20 2021, 3:53 AM

@Gopavasanth Thanks.

  • I am not sure why the font is affected. I think it is related to dark/light theme toggle and not the progress bar. I can fix that in a separate commit.
  • Timing is the current video time being processed. I think it is in HH:MM:SS.MS format which taken directly from ffmpeg output. I can change it if needed but I think this format is more suitable.
  • I purposely made trim comes after other operations because if there are multiple trims the other operations will cause more performance issues (each trim will be rotated, cropped .. etc, instead of occurring in one step). I did some research to see if trim can be combined with other operations using ffmpeg but could not find any results.
  • Rotation will cause the resultant video to be displayed that way. Nothing to do with the current changes. I can fix this in a separate commit.
  • The changes I added are not breaking. Everything works as it was before with no additional requirement. I only refactored the code. I documented my code as much as possible. I added docblock to each function and comments to where the code seems ambiguous. Otherwise I used clear variable names which should be good indication for future developers.
Gopavasanth added a comment.Mar 20 2021, 4:10 AM

Awesome!

Timing is the current video time being processed. It is in HH:MM:SS:MS format. What format did you want to change it? The one you suggested is the same as the current one.

Ah yeah, Is it being shown currently when you tried locally?

I purposely made trim comes after other operations because if there are multiple trims the other operations will cause more performance issues (each trim will be rotated, cropped .. etc, instead of occurring in one step). I did some research to see if trim can be combined with other operations using ffmpeg but could not find any results.

Good point, how about having both the methods based on user choices i.e multi trim and single trim, so we may change the rest of the flow for better performance? As if the user trims a video from 1min-10min in a 60min video + Rotate/perform other options, the rest of the encodings are pointless to perform on entire 60min video right and then triming to 10min.

Khr2003 added a comment.Mar 20 2021, 4:19 AM

Ah yeah, Is it being shown currently when you tried locally?

I edited my comment before your added the reply.

Good point, how about having both the methods based on user choices i.e multi trim and single trim, so we may change the rest of the flow for better performance? As if the user trims a video from 1min-10min in a 60min video + Rotate/perform other options, the rest of the encodings are pointless to perform on entire 60min video right and then triming to 10min.

That makes sense. I will look into that.

Gopavasanth added a comment.Mar 20 2021, 4:29 AM

I edited my comment before your added the reply.

No worries, I just saw your edited text. I think it makes sense, agreed.

The changes I added are not breaking. Everything works as it was before with no additional requirement. I only refactored the code.

Ah I was meaning these are really nice and stunning updates xD. Also, sorry I didn't go through the all files in your code changes before, super excited to try out the first so tested the tool locally, just going through your code comments in the code. those are damn great. thank you!

Khr2003 added a comment.Mar 21 2021, 4:17 AM

Ah I was meaning these are really nice and stunning updates xD. Also, sorry I didn't go through the all files in your code changes before, super excited to try out the first so tested the tool locally, just going through your code comments in the code. those are damn great. thank you!

Thanks, All good.

I was able to find a way to combine trim with other manipulations. Now if user select trim changes will occur at the same time (trim, rotation, cropping .. etc). I submitted a new patchset.

gerritbot added a comment.Mar 21 2021, 10:47 AM

Change 673486 merged by jenkins-bot:
[labs/tools/VideoCutTool@master] Added progress bar and refactored workers codebase

https://gerrit.wikimedia.org/r/673486

gerritbot added a comment.Mar 21 2021, 10:47 AM

Change 673487 merged by jenkins-bot:
[labs/tools/video-cut-tool-back-end@master] Added endpoints for progress update

https://gerrit.wikimedia.org/r/673487

gerritbot added a comment.Mar 21 2021, 10:47 AM

Change 671933 merged by jenkins-bot:
[labs/tools/video-cut-tool-worker@master] Format and refactor codebase

https://gerrit.wikimedia.org/r/671933

Gopavasanth mentioned this in rLTVW5e11560cabd4: Format and refactor codebase.Mar 21 2021, 10:47 AM
Gopavasanth mentioned this in rLTVT0613d5e437f6: Added progress bar and refactored workers codebase.Mar 21 2021, 11:00 AM
Gopavasanth mentioned this in R2574:12248088fe75: Added endpoints for progress update.
Maintenance_bot removed a project: Patch-For-Review.Mar 21 2021, 11:10 AM
Gopavasanth awarded a token.Mar 21 2021, 11:56 AM

Thanks for the great work! Now it's live https://videocuttool.wmflabs.org/ \o/!!!

Khr2003 added a comment.Mar 21 2021, 12:10 PM

thanks for you help to fix the remainder of the issues. It is awesome to see live after all that work :)

Gopavasanth mentioned this in T243589: VideoCutTool pending tasks to release version 0.2.Mar 21 2021, 4:42 PM
Gopavasanth closed this task as Resolved.Mar 24 2021, 4:19 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits