rosbag2_transport: playback rate control using keyboard shouldn't simply multiply current rate by 1.1 or 0.9

[xqrdot]

Description

When playing back a rosbag, there is a possibility to control the rate of the playback using keyboard arrows: UP ARROW increases the playback rate by 10%, and DOWN ARROW decreases by 10%. But this feature is unintuitive to use, as 10% is taken/added to the currently set rate. It makes it hard to adjust the rate, for example, from 100% rate to 200%. Also, it is impossible to go back to the last value (i.e. going from 100% to 90% and increasing the speed back up, gives 99% where 100% is expected).

Expected Behavior

The playback rate would be controlled either in fixed steps (10%, or 5% increments) or in fixed steps in relation to the initially defined playback rate.

Actual Behavior

Link to the code in question: https://github.com/ros2/rosbag2/blob/rolling/rosbag2_transport/src/rosbag2_transport/player.cpp#L786-L791

The modification of the playback rate takes into account only the current playback rate and simply multiplies it by either 1.1 or 0.9.

To Reproduce

1. Start playing back a rosbag.
2. Decrease playback rate by 10% (press DOWN ARROW once).
3. Increase playback rate by 10% (press UP ARROW once).
4. Observe that the playback rate is reported as 0.99 (99%), where 1.0 (100%) is expected.

System (please complete the following information)

• OS: Ubuntu 22.04
• ROS 2 Distro: Dashing
• Version: rolling

Additional context

I'd like to fix this myself. Please advise on whether to control the speed using steps (IMO, the "easier" approach) or to modify the playback rate by 10% based on the initially set playback rate (by arguments)

[MichaelOrlov]

@xqrdot Thanks for filing this bug report.
I have to admit that yes we have this wrong behavior and it seems I missed it on code review.

As regards to the fix proposal it's going to be like "Hold my beer, I have done it before" 😄
In player in following places:

rosbag2/rosbag2_transport/src/rosbag2_transport/player.cpp

Lines 784 to 793 in c7c7954

	add_key_callback(
	play_options_.increase_rate_key,
	[this]() {set_rate(get_rate() * 1.1);},
	"Increase Rate 10%"
	);
	add_key_callback(
	play_options_.decrease_rate_key,
	[this]() {set_rate(get_rate() * 0.9);},
	"Decrease Rate 10%"
	);

Instead of multiplying current rate by factor 1.1 and 0.9 need to add 0.1 and subtract 0.1 respectively in relevant cases.

PR as usually is welcome, please don't forget to add or change tests to cover changes and to avoid regression in future.