Ymagis / ClairMeta

Clairmeta is a python package for Digital Cinema Package (DCP) probing and checking.
BSD 3-Clause "New" or "Revised" License
84 stars 22 forks source link

Improve progress callback #154

Closed remia closed 4 years ago

remia commented 4 years ago

Seeings as your kicking around in that code. Another small extra I would like is for .. def console_progress_bar(file_path, progress, elapsed, done) be enhanced to have def console_progress_bar(file_path, progress_dcp, dcp_size, progress_file, file_size, elapsed, done) So you can display overall progress, file progress and ETA for completion based on how long its been taking to process data up until that time.

Originally posted by @jamiegau in https://github.com/Ymagis/ClairMeta/issues/152#issuecomment-667939856

jamiegau commented 4 years ago

Note, I addressed this implementation in the following pull request,

157

This version will display two different complete percentages. One for the full DCP and one for the current file.

remia commented 4 years ago

Hey @jamiegau,

Could you try to keep only the changes to the progress callback in that PR so that I can start testing it ? It currently have fix for subtitles related errors that I think are not definitive yet so they should probably lives in their own PR.

Thanks for the contribution !

PS: you can rebase against develop, CI checks should be passing now.

jamiegau commented 4 years ago

I have only ever written software for my own needs. I am hopping getting involved in helping here will help with my code quality and working with others..

Can I ask, I take it, I fork the repo.. then create a branch for each issue I want to work on, and then apply that branch as a pull request against a specific bug/feature. (This is how I have noticed in helping on this project.) I am new to using git like this. I will follow this path going forward..

remia commented 4 years ago

Yes that sounds good.

You can even use interactive rebase and force push to update the PR in place but that's a little more involved. As you already closed the PR, I'll just wait for the new one.

remia commented 4 years ago

Just updating what was discussed:

jamiegau commented 4 years ago

Yes, in regards to updating the progress feature.

remia commented 4 years ago

Merged in #162