Skip to content

feat(windows): Add millisecond level precision when seeking#1548

Closed
SNNafi wants to merge 1 commit into
bluefireteam:mainfrom
GreentechApps:feat/windows-millisecond-level-precision
Closed

feat(windows): Add millisecond level precision when seeking#1548
SNNafi wants to merge 1 commit into
bluefireteam:mainfrom
GreentechApps:feat/windows-millisecond-level-precision

Conversation

@SNNafi

@SNNafi SNNafi commented Jun 16, 2023

Copy link
Copy Markdown
Contributor

Description

This pull request addresses the issue related to audio seeking precision in the Audioplayers plugin for Windows. The problem is currently limited to seconds instead of milliseconds. The proposed changes modify the code to support seeking with millisecond precision, allowing users to accurately seek to specific points within an audio file.

Changes Made:

File: MediaEngineWrapper.cpp
Modified the SeekTo function:
Changed the parameter type from uint64_t to double for the timeStamp parameter.

Updated the calculation of timestampInSeconds to convert timeStamp from milliseconds to seconds (timeStamp / 1000.0).

Changed the call to m_mediaEngine->SetCurrentTime to use the updated timestampInSeconds value.

Modified the GetMediaTime function:
Removed the unnecessary assignment of currentTime in milliseconds.

Updated the calculation of currentTimeInSeconds to convert the current time from seconds to milliseconds (currentTimeInSeconds * 1000.0).

Return only currentTimeInHns instead of both currentTimeInHns and currentTime.

Modified the GetDuration1 function: Updated the calculation of durationInSecondsto convert the duration from seconds to milliseconds(durationInSeconds * 1000.0). Return duration`.

File: audio_player.cpp
Modified the OnTimeUpdate() method:
Updated the flutter::EncodableValue for the value field to directly use (int64_t)m_mediaEngineWrapper->GetMediaTime() instead of dividing by 10000.

Modified the OnDurationUpdate() method:
Updated the flutter::EncodableValue for the value field to directly use (int64_t)m_mediaEngineWrapper->GetDuration() instead of dividing by 10000.

Modified the SeekTo function in the AudioPlayer class to accept a double parameter instead of int64_t.

File: audio_player.h
Updated the function declaration of SeekTo in the AudioPlayer class to accept a double parameter instead of int64_t.

File: audioplayers_windows_plugin.cpp
Modified the seek method to use double for the position argument and directly pass it to player->SeekTo.

Checklist

  • The title of my PR starts with a [Conventional Commit] prefix (fix:, feat:, docs:, chore: etc).
  • I have read the [Contributor Guide] and followed the process outlined for submitting PRs.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation and added dartdoc comments with ///, where necessary.
  • I have updated/added relevant examples in [example].

Breaking Change

  • Yes, this is a breaking change.
  • No, this is not a breaking change.

Related Issues

#1547

@spydon spydon requested a review from Gustl22 June 26, 2023 08:55
@Gustl22

Gustl22 commented Jun 26, 2023

Copy link
Copy Markdown
Collaborator

I would rather prefer to use double precision seconds everywhere. The conversion from milliseconds ms and hundredMilliSeconds Hns and seconds is quite confusing in my opinion. @SNNafi if you agree, I will push on top of it, with the improvements.

@spydon

spydon commented Jun 26, 2023

Copy link
Copy Markdown
Member

I would rather prefer to use double precision seconds everywhere. The conversion from milliseconds ms and hundredMilliSeconds Hns and seconds is quite confusing in my opinion. @SNNafi if you agree, I will push on top of it, with the improvements.

That sounds like a great improvement to code readability!

@SNNafi

SNNafi commented Jun 26, 2023

Copy link
Copy Markdown
Contributor Author

Thanks, @spydon for approving.
@Gustl22, Yes, please. It will be much better.

@Gustl22

Gustl22 commented Jun 26, 2023

Copy link
Copy Markdown
Collaborator

@SNNafi I'm not allowed to make edits to your Pull request, if you wish so, you have to enable that option on the right of your Pull request.

@SNNafi

SNNafi commented Jun 26, 2023

Copy link
Copy Markdown
Contributor Author

@Gustl22, It seems GitHub doesn't allow "Allow edits by maintainers" option, if PR is given from an org account. :(

@spydon

spydon commented Jun 26, 2023

Copy link
Copy Markdown
Member

@Gustl22 maybe we could just merge this to another branch than main and then you can continue with the remaining fixes on that branch?

@Gustl22

Gustl22 commented Jun 27, 2023

Copy link
Copy Markdown
Collaborator

Closing in favor of #1553, @SNNafi plz have a look, if it fulfills your needs ;D

@Gustl22 Gustl22 closed this Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants