Update for compatibility with pose2sim

[AurelienCoppee]

Rename (and adapted the code) to move from time_range to frame_range, allowing compatibility with pose2sim

[davidpagnon]

Hi @AurelienCoppee,

Thank you for your suggestion! I have been endlessly debating whether to use a time_range or a frame_range. A frame_range is more precise, but much less intuitive than a time_range. And Sports2D is meant to be as simple as possible.

Now that I think about it, it would be nice to have both options available. Would you mind trying to implement that? In case both parameters are given, we can print a warning message like so:
logging.warning(f'Both time_range and frame_range are provided. Only frame_range = {frame_range} will be taken into account.')

It would be great if you could test it both with a config file where neigher time_range nor frame_range are empty lists [ ]:
sports2d --config path_to_config.toml
and with arguments:
sports2d --time_range 0 2.1 --frame_range 5 30

Thanks in advance!

[davidpagnon]

Thanks! I'll check it out in a little bit, I'm afraid I can't find time right now.

[AurelienCoppee]

No problem, your feedback is welcome, I continue to look at how to improve their interoperability.
Instead of selecting one of them in the case where the two are mentioned, I stop the process and print a log. I think that continuing the process will hide the log and lead to hidden issues for the user

[davidpagnon]

Hi @AurelienCoppee,

Could you give me a summary of what you edited and why, so that it would help the reviewing process?
Also, can you tell me when you think you're at a good stopping point so that I do not do multiple reviews?

Finally, the checks fail but it is likely to be due to you changing show_realtime_results = true to display_detection = true. There is no screen on the server running the checks so we need to disable display, which I did there: but now you also need to edit the key in the tests.py file.
By the way, why did you change that? I'm not necessarily against but do you think it is clearer this way? The problem is that in the jargon of the field, detection corresponds to the detection of the bounding box, while our results also estimate pose and angles.

[AurelienCoppee]

Hi,

I basically only renamed and reorganized the code to achieve easy integration of sports2d in pose2sim.
I already started the mutualisation of function in my last commits, which is already an integration work. There's still work to do but I did some breaking changes by renaming variables so I think it may be interesting to integrate them (after review) to avoid conflicts. Maybe a new branch for a new version may be a possibility?

It's the name used in pose2sim for the "same" purpose, i don't have knowledge in your field, I'm a software dev, so it's only a suggestion for a more comprehensive code.

[davidpagnon]

It's the name used in pose2sim for the "same" purpose, i don't have knowledge in your field, I'm a software dev, so it's only a suggestion for a more comprehensive code.

Oh yes you are right 😅 I guess I was the one giving it a bad name actually, it would make more sense to call it show_realtime_results in Pose2Sim as well (and make it backwards compatible by accepting both show_realtime_results and display_detection from Config.toml in Pose2Sim).

I did some breaking changes by renaming variables so I think it may be interesting to integrate them (after review) to avoid conflicts. Maybe a new branch for a new version may be a possibility?

I'm not sure I understand and answer correctly, but maybe we could just make it a new release (v0.5) once it is done, and document the change? I appreciate you trying to make the changes necessary for an integration into Pose2Sim by the way, I've wanted to do it for a while! I have the project of adding the possibility of calibrating the camera for coordinates in meters, and to run 2D inverse kinematics. But I'll have to postpone it for now :)

[AurelienCoppee]

I'm doing the changes on Pose2Sim also, should i print a log warning peoples to moves from display detection to show_realtime_results if they're using it to prepare them to future remove ? Can also do it for the variables I changed in sports2D

Yes it's a good idea, i was just suggesting to create a new branch (Where I will do my next pull request) so instead of this pull request the code is on a place everyone can edit.
I need it for a project so it's a pleasure to help!

[davidpagnon]

I'm doing the changes on Pose2Sim also, should i print a log warning peoples to moves from display detection to show_realtime_results if they're using it to prepare them to future remove ? Can also do it for the variables I changed in sports2D

That would be nice! By the way, can you still run Sports2D with a config file, and from within Python?

I just created a branch "experimental", it may be a good idea indeed!

[davidpagnon]

Oh okay I misunderstood, thank you for creating the branch on your side

[AurelienCoppee]

No you understood well on the first time 😂 Changed the destination of this pull request

By the way, can you still run Sports2D with a config file, and from within Python?

Didn't understood what you meant?

[davidpagnon]

You can run Sports2D different ways:

• With parameters: sports2d --video_input path_to_video1.mp4 path_to_video2.mp4 --other_param value
• With a config file: sports2d --config path_to_config.toml
• Within a python script: from Sports2D import Sports2D; Sports2D.process('Config_demo.toml')

Does it still work in all cases?

[AurelienCoppee]

Yes it does.

I also changed the parameters to_image and to_video to save_video, the same way it works on pose2sim. What do you think about that? I didn't already added a backward compatibility

[AurelienCoppee]

(I edited the previous comment about the save_video, not sure you saw it)

[davidpagnon]

Sorry, yes I think it makes sense!

[AurelienCoppee]

No problem, ok nice!