[audioplayers] Fix resource cleanup bugs #871

seungsoo47 · 2025-07-15T10:46:10Z

Handled player resource cleanup in destructor
Renamed Release() to ReleaseMediaSource() for clearer responsibility
Handled errors by replacing exceptions in GetDuration() and GetPosition()
Added logging for unsupported features
Fixed range limitation of SetVolume() and SetPlaybackRate()

- Handled player resource cleanup in destructor - Renamed Release() to ReleaseMediaSource() for clearer responsibility - Handled errors by replacing exceptions in GetDuration() and GetPosition() - Added logging for unsupported features - Fixed range limitation of SetVolume() and SetPlaybackRate()

packages/audioplayers/tizen/src/audioplayers_tizen_plugin.cc

packages/audioplayers/tizen/src/audio_player.cc

JSUYA · 2025-07-16T06:21:08Z

packages/audioplayers/tizen/src/audio_player.cc

+  // TODO(seungsoo47): The player_set_playback_rate() API has a limitation of
+  // 0.5-2x on TV and is not supported on RPI.
+  if (playback_rate < 0.5) {
+    playback_rate = 0.5;
+  } else if (playback_rate > 2.0) {
+    playback_rate = 2.0;
+  }


According to API guide...
It looks like playback_rate should accept values less than 0 and greater than 2.0. In particular, it looks like values less than 0.5 are also possible. Can you confirm?

Thank you for the review. As I wrote in the comment, my test results show that the TV only supports 0.5-2.0, unlike the specifications.

my test results show that the TV only supports 0.5-2.0, unlike the specifications.

Does your test return PLAYER_ERROR_INVALID_* when the rate value is below 0.5 and above 2.0?

This API is also available in the RPI environment (Common profile), so we cannot consider only the TV case.
If an invalid value is provided, PLAYER_ERROR_INVALID_PARAMETER or PLAYER_ERROR_INVALID_OPERATION will be returned, so this can be confirmed in the error log.
Therefore, if values between 0.0 and 0.5 do not work, this should be confirmed through the error return value.

According to API guide, values below 0.0 and above 2.0 will be muted. I don't think this means that the API is not working. According to the internal API reference related to the TV API, the range of rates that can be assigned varies depending on the resource type.
Therefore, I don't think it is necessary to limit the minimum and maximum values.

I think the same applies to volume.
If entering a value outside the range causes a system crash or serious problem, it is possible to limit that value, but if it is already being handled within the platform API, I don't think there is any need to limit it.

What do you think?

Thanks for your comments.
As I wrote in the comments, the API does not work on the RPI environment.
And iOS/MacOS support the value between 0.5 ~ 2, too.
https://github.com/bluefireteam/audioplayers/blob/f2269aeb270154e5f3571e1e025d340cfbca4686/packages/audioplayers/lib/src/audioplayer.dart#L338
So I think it's not a bad idea to preprocess it in native code.
Also, if there are no big problems, wouldn't it be better to just remove unnecessary errors?
If the error persists, other developers may investigate the error again.

The issue of not working when the rate value is between 0.0 and 0.5 is an API issue. Therefore, I think it a valid value at the plugin level.
It is not certain that it will work the same on all TV models(All tizen versions). and , if a problem occurs, it will be expressed as an error log.
If you wish to provide a warning to users, you can use the limitations in the readme.
(The same applies to cases where it does not work on rpi.)

If the value exceeds the range of 0.0 to 2.0, this will cause an error in the Native API, so there is no need to modify this value in the plugin.

If the user gives a value of 3.0 and then checks the rate value again, 3.0 will be returned.
However, since the value is actually converted to 2.0, this results in an outcome different from the user's intention.
https://github.com/bluefireteam/audioplayers/blob/main/packages/audioplayers/lib/src/audioplayer.dart#L341

If a warning is necessary, I recommend outputting a separate log message.

if (playback_rate < 0.0 || playback_rate > 2.0) { //warning ... }

However, if you still think it is correct to change the value, I recommend adding the URL of this PR to the comments and showing the change in the log when the value is changed.

packages/audioplayers/tizen/src/audio_player.cc

JSUYA

and please update CHANGELOG and version.

JSUYA · 2025-07-17T02:02:34Z

packages/audioplayers/tizen/src/audio_player.cc

+  // TODO(seungsoo47): The player_set_playback_rate() API has a limitation of
+  // 0.5-2x on TV and is not supported on RPI.
+  if (playback_rate < 0.5) {
+    playback_rate = 0.5;
+  } else if (playback_rate > 2.0) {
+    playback_rate = 2.0;
+  }


my test results show that the TV only supports 0.5-2.0, unlike the specifications.

Does your test return PLAYER_ERROR_INVALID_* when the rate value is below 0.5 and above 2.0?

This API is also available in the RPI environment (Common profile), so we cannot consider only the TV case.
If an invalid value is provided, PLAYER_ERROR_INVALID_PARAMETER or PLAYER_ERROR_INVALID_OPERATION will be returned, so this can be confirmed in the error log.
Therefore, if values between 0.0 and 0.5 do not work, this should be confirmed through the error return value.

According to API guide, values below 0.0 and above 2.0 will be muted. I don't think this means that the API is not working. According to the internal API reference related to the TV API, the range of rates that can be assigned varies depending on the resource type.
Therefore, I don't think it is necessary to limit the minimum and maximum values.

I think the same applies to volume.
If entering a value outside the range causes a system crash or serious problem, it is possible to limit that value, but if it is already being handled within the platform API, I don't think there is any need to limit it.

What do you think?

JSUYA · 2025-07-17T02:06:27Z

packages/audioplayers/example/lib/tabs/controls.dart

@@ -96,7 +96,7 @@ class _ControlsTabState extends State<ControlsTab>
        ),
        WrappedListTile(
          leading: const Text('Rate'),
-          children: [0.0, 0.5, 1.0, 2.0].map((it) {
+          children: [0.5, 1.0, 2.0].map((it) {


I think it would be better to use the same example as audioplayers.
https://github.com/bluefireteam/audioplayers/blob/main/packages/audioplayers/example/lib/tabs/controls.dart#L104
If this value is limited, I think it should be passed as an error through the platform code.
(There is no need to change it unless there are problems running or building the example, or unless it is platform-dependent code.)

I think we need to change it rather than causing unnecessary errors.
What do you think?

(There is no need to change it unless there are problems running or building the example, or unless it is platform-dependent code.)

It is recommended to maintain code synchronization with the original plugin example for code update management.
Users can see how the behavior differs on each platform through the same example. Of course, not all of our examples maintain the same code, but I think it's good to reduce the differences as much as possible.

github-actions bot added the p: audioplayers label Jul 15, 2025

Remove unnecessary_breaks rule

36f830c

JSUYA self-requested a review July 16, 2025 05:27

JSUYA reviewed Jul 16, 2025

View reviewed changes

seungsoo47 force-pushed the dev.plugins.audioplayers.fix.250715 branch from 052a1e5 to 97abe00 Compare July 16, 2025 09:15

Apply suggestions from code review

7e1508f

seungsoo47 force-pushed the dev.plugins.audioplayers.fix.250715 branch from 97abe00 to 7e1508f Compare July 16, 2025 09:21

Update changelog and version

8f333f3

JSUYA reviewed Jul 17, 2025

View reviewed changes

JSUYA mentioned this pull request Jul 18, 2025

[video_player_videohole] Automatically rotates video player based on device orientation #872

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[audioplayers] Fix resource cleanup bugs #871

[audioplayers] Fix resource cleanup bugs #871

Uh oh!

seungsoo47 commented Jul 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JSUYA Jul 16, 2025

Uh oh!

seungsoo47 Jul 16, 2025 •

edited

Loading

Uh oh!

JSUYA Jul 17, 2025

Uh oh!

seungsoo47 Jul 17, 2025

Uh oh!

JSUYA Jul 17, 2025

Uh oh!

Uh oh!

Uh oh!

JSUYA left a comment

Uh oh!

JSUYA Jul 17, 2025

Uh oh!

JSUYA Jul 17, 2025

Uh oh!

seungsoo47 Jul 17, 2025

Uh oh!

JSUYA Jul 17, 2025

Uh oh!

Uh oh!

[audioplayers] Fix resource cleanup bugs #871

Are you sure you want to change the base?

[audioplayers] Fix resource cleanup bugs #871

Uh oh!

Conversation

seungsoo47 commented Jul 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seungsoo47 Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JSUYA left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

seungsoo47 Jul 16, 2025 •

edited

Loading