Add in/out point support to ola_recorder #1648
Conversation
|
Just for info, I started on #1607 which I think actually covers the specifics of #1604 but then got sidetracked with various stuff. It looks like we've actually worked on different features, which is great, there's just a small chance of a merge conflict when they both go in. I'll try and get mine merged in soon to simplify that. The other features look good and are certainly likely to be wanted by people. |
|
Worst case I can try to rebase this once #1607 is merged. I did a bit of refactoring to clean up how show files are read; I imagine that's the most likely place for a merge conflict. |
|
I've switched |
peternewman
left a comment
peternewman
left a comment
There was a problem hiding this comment.
A few more comments, mostly style changes
peternewman
left a comment
peternewman
left a comment
There was a problem hiding this comment.
Sorry a few more minor comments now I've been able to review the whole thing en-mass since it's finally settled down and is pretty much finished.
| OLA_WARN << "Show file ends before the stop time (actual length " | ||
| << m_playback_pos << " ms)"; | ||
| if (loop) { | ||
| const uint64_t remaining_time = m_stop - m_playback_pos; |
There was a problem hiding this comment.
I think I was more worried from a maths perspective of having 2 - 5 = trouble!
| if (m_iteration_remaining > 0) { | ||
| OLA_WARN << "More iterations requested than can fit in the duration " | ||
| "(" << m_iteration_remaining << " iteration(s) remain)"; | ||
| } |
There was a problem hiding this comment.
Should this not go in verify instead?
There was a problem hiding this comment.
Given that other usage-type problems (e.g. stop point beyond show file end) are warned during playback, this seemed a fitting place for the warning.
There was a problem hiding this comment.
Hmm yeah, can I say both? 😄 It would be nice to spot config issues during validation, rather than needing to wait for a 30 minute show to run, but it would also be good to see them during show run...
There was a problem hiding this comment.
Because the verify function is the player without output, it appears in both places. For instance, with a recorded show file about 11 seconds long:
ola_recorder --verify recorded.show --iterations 3 --duration 1
------------ Summary ----------
For all (3) iterations:
After playing for 1 second(s) total:
Universe 1: 4 frames
Universe 2: 40 frames
Total frames: 44
Total playback time: 1 seconds
examples/ShowPlayer.cpp:130: More iterations requested than can fit in the duration (3 iteration(s) remain)
There was a problem hiding this comment.
Ah, I forgot, and got confused by your comment about it being during playback! If I was to nit-pick, it would be nice if the warnings came before the summary, but no worries if that's not easily possible.
peternewman
left a comment
peternewman
left a comment
There was a problem hiding this comment.
Thanks again for all your work on this @DragoonBoots , sorry it became a bit of a beast!
|
I realised I was being a bit petty with a few bits sorry, so I've merged it. If you fancy trying to tidy them up at some point then great, otherwise I'll try and get to them eventually. Thanks again! |
2 participants
Fixes #1604.
This seemed like a good way to learn more about OLA's event handling.