Skip to content

Conversation

@eruffaldi
Copy link

  1. offset function in Atom
  2. test.py accepts files and directories
  3. example of extraction of frame duration times super quick vs classic approaches

@SteveMarshall
Copy link
Owner

Hi @eruffaldi, this is really interesting, thanks! It's a bit hard for me to tell what the intent of the work is, though. Could you rework the commits so they follow Tim Pope's guidelines? That way, I'll be better able to see what each commit (and the overall PR) does and review it properly.

@eruffaldi
Copy link
Author

I will re-submit the Pull Request with proper commits.

You can see what I have done with the patch in this blog post: http://teslacore.blogspot.it/2017/05/script-extract-mp4-effective-durations.html

@SteveMarshall
Copy link
Owner

@eruffaldi Were you still planning to rebase this pull request, by any chance? It'd be really good to get your work into a state where I can review and merge it!

@eruffaldi
Copy link
Author

eruffaldi commented Feb 9, 2018

Dear Steve, I have squashed the commits via rebase:

  • atom.py modified
  • setframesduration.py script
  • getframesduration.py script
  • test.py script modified for supporting path spec

@SteveMarshall
Copy link
Owner

Hi Emanuele, as I requested in my comment on May 15th, could you rebase the commit so it's a series of smaller commits, each with clear messages that follow the guidelines I linked to?

Each commit should be one self-contained, clear, change with a message explaining why that change has been made. As it stands, I can sort of see what you're doing, but I don't really understand why. (Your blog post sort-of explains, but I'd like to understand why you've made each change or addition you have.)

- expose offset and size of frames, add method for data field built from StringIO
- getframedurationutility
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants