search

frankier / skelshop

📺 📰 🧑‍💼 Toolkit for skeleton & face analysis of talking heads (e.g. news) videos 🧑‍💼 📰 📺
https://frankier.github.io/skelshop
MIT License
5 stars 1 forks source link

framerate using ffmpeg, fix wrong hand-start-index #14

Closed cstenkamp closed 3 years ago

frankier commented 3 years ago

Looks good to me! What was the problem with OpenCV's FPS? Can you tell me when it does/doesn't work versus ffprobe?

A few comments which if you agree you can just fix straight onto master (I've invited you as a collaborator to this repo but I don't think you accepted yet) --- or ignore if you have other priorities:

  1. Maybe a bit too fussy to even mention, but could we not put in editor specific stuff like "# \<editor-fold desc="click-decorators">"?
  2. If you like you can go ahead and add ffprobe to the docker/Singularity image.
  3. Yes it looks like there's no difference in openpose_multi. This was just a copy paste to fix a mistake where I'd thought the hand wrist keypoints were the same as the ones in BODY_25.
  4. It's a "what could actually go wrong" situation, but it might be better to avoid running eval() on ffprobe output and just do the divide ourselves.
frankier commented 3 years ago

I've addressed these points now I think. There were also some style + typing problems which can be checked by running ./run_checks.sh . You can make sure they're always checked by installing the pre-commit hook pip install --user pre-commit && pre-commit install

frankier commented 3 years ago

P.S. I moved the ffprobe option to the top level since multiple subcommands need it.

frankier commented 3 years ago

Thinking about this again, was the problem that your opencv was not linked against ffmpeg? If it's linked against ffmpeg I think it has the same information as ffprobe. Not linking against ffmpeg might cause problems too, e.g. slow playback + less formats supported.