Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Audio message recorder and player functionality implemented #615

Closed
wants to merge 11 commits into from

Conversation

sukhman-sukh
Copy link

@sukhman-sukh sukhman-sukh commented Mar 5, 2024

Added voice chat feature.
Now, anyone can record a message from qaul application from the custom input area and send a voice message using libqual_worker and play the voice message directly from the application.

Issue Resolved :- Record and Send Voice Messages . fix #548

@sukhman-sukh
Copy link
Author

I am so sorry. I messed up something with my forked repo so an earlier one got closed and had to reopen a new one.
@MathJud @brenodt.
Regards.

@brenodt
Copy link
Collaborator

brenodt commented Mar 12, 2024

Hi @sukhman-sukh , thanks for the PR!
I'm co-authoring it so we can move it closer to a complete feature. Moving this to draft, as it's not ready to be merged just yet.

There are still some tasks that I need to take care of prior merging, but your help in getting the ball rolling and jump-starting the feature's foundation is much appreciated.

A few things done so far:

  • rebased the branch so it compiles the Flutter app
  • Updated the UI in the audio message so it's a bit closer to the design language of the app; previews on MacOS:
dart light
  • Added missing configuration for iOS, MacOS, Android
  • Some minor code refactoring so that the code follows the structure we use across the codebase

I was able to test sending an audio message from a MacOS host, which worked.

Still pending:

  • sending from an Android host - Unable to do so on my Moto G7, Android 10; I'm getting a crash
  • sending from an iOS host
  • sending from a Windows host (@MathJud going to need your help here)

Pending definition:

@MathJud do we want Linux support? As per the record package documentation, it relies on fmedia being installed separately.

If we opt to add Linux support, we must also add that dependency to the snap package.

@brenodt brenodt self-assigned this Mar 12, 2024
@sukhman-sukh
Copy link
Author

Hey @brenodt,
Thanks for reviewing the PR.
At each step, I tested the feature on IQOO 9 pro, Android version 14 and it was working. Still, I will check for the reason of its crash on other versions as well.
Can you tell me the steps to reproduce the error?

WhatsApp Image 2024-03-13 at 1 34 27 AM

This is a screenshot of the android qaul application until commit #478a8e2.

@sukhman-sukh
Copy link
Author

I will have a look on how to extend this to Windows, Linux, and IOS hosts.

@MathJud
Copy link
Member

MathJud commented Mar 20, 2024

Android

I tested the feature on Android 11 & 12, and it worked fine for me.

@MathJud
Copy link
Member

MathJud commented Mar 20, 2024

Windows

I tested the feature on windows, and it was not active.
It even crashed the opening of the voice file in the app, due to a wrong path:

flutter: [SEVERE]       (2024-03-20 12:01:13.402035): PlatformException(open_error, Failed to open file:///C:/Users/jud/AppData/Roaming/qaul/qaul/config/logs/12D3KooWReEQFc1Upz7bWaLQGj7pud261f7TvNja723BNHExk2TY/files/6383124272380269290.m4a: ShellExecute error code 2, null, null)

qaul-ui-audio-windows

@sukhman-sukh
Copy link
Author

Oh!
Yeah the issue might be the voice file path
Fixing it.
Also how to test for iOS device(I don't have test iOS device)?

@brenodt
Copy link
Collaborator

brenodt commented Apr 2, 2024

Hi @sukhman-sukh , I was able to verify that it works on iOS, so we're clear on that front!
I also made some small UI changes to improve the overall usability of the feature.

As it stands, there are two pending tasks here:

  • Test feature on Windows
  • Test feature on Linux

As soon as we're able to confirm those, this will be eligible for merging.
If you need any help, let me or @MathJud know!

@sukhman-sukh
Copy link
Author

Hey @brenodt,
Thanks for reviewing.
I was testing this for Windows and found out that, the "onPickImagePressed" widget does not work for Windows and was disabled for it. Since the audio widget is also based on the same implementation, it is also broken for Windows.
Screenshot 2024-04-06 044625
Image sent to Windows-qaul from the mobile application. (Tested for both main and audio-chat branches).

Should we create this as a separate issue and fix it or continue in this one only?

Thanks.

@brenodt
Copy link
Collaborator

brenodt commented Apr 8, 2024

Hi @sukhman-sukh , from what I recall, the reason onPickImage is disabled on Desktop is that "image picking" as a user flow is mobile only. On desktop, we pick files to send as attachments, since the application can access the folder structure more easily.

In any case, I don't think this would affect the scope of this PR, since both packages we added as deps to enable this feature support Windows:

You just need to enable the audio button on Windows (qaul_ui/lib/screens/home/tabs/chat/widgets/chat.dart:323), which will allow you to test it.

@sukhman-sukh
Copy link
Author

sukhman-sukh commented Apr 8, 2024

Yes, I see that and I am testing it on Windows . The error I described above is not about sending. It is even when I receive any attachment/audio/image in qaul.
Above is the screenshot of the onMessageTap on the received image. It is failing to locate the item on device for any type of file transferred.
It is trying to locate it './AppData\Roaming\qaul\qaul\config\logs/12D3KooWA9XssRbjZ1Daqs2BKqCq9w5Y617i8HG9cwqKnu6171za/files/17685287422266572378.jpg' here whereas Image is present at './AppData\Roaming\qaul\qaul\config\12D3KooWA9XssRbjZ1Daqs2BKqCq9w5Y617i8HG9cwqKnu6171za/files/17685287422266572378.jpg'.

Currently, I am working on the issue and I hope, I will finish debugging it ASAP.

@brenodt
Copy link
Collaborator

brenodt commented Apr 8, 2024

Oh, I see. Cool, let me know if you need any help!

@sukhman-sukh
Copy link
Author

Hi @MathJud @brenodt ,
I have fixed the Windows ChatFile file_path parsing issue.

Qaul_test_passed
Screenshot of Qaul chatroom running on Windows.

Please review.
Thanks

@brenodt
Copy link
Collaborator

brenodt commented Apr 9, 2024

Great! Glad to hear it, @sukhman-sukh .
I'm honestly not certain why are attachments being sent to/stored in the logs folder, but it shouldn't block the landing of this feature - I'll take a look at it later.

Are you able to take a look to confirm it's working on Linux, also?

@sukhman-sukh
Copy link
Author

Hi @brenodt,
Yes, I tested this for linux and it is working.
image

Also, enabling audio-chat feature for linux as well.

@MathJud
Copy link
Member

MathJud commented Apr 11, 2024

@sukhman-sukh thanks a lot for your work!
In general I think we are ready to merge.
I would like to take a few more days for testing this feature on all platforms before merging.

@MathJud MathJud marked this pull request as ready for review April 11, 2024 16:37
- `Visual Studio 2022` is now required due to the required CMAKE version.
- the flutter build path to copy `libqaul.dll` was corrected
@MathJud
Copy link
Member

MathJud commented Apr 12, 2024

As this branch had merge conflicts I rebased it to the current main branch and force pushed it here again.

@MathJud
Copy link
Member

MathJud commented Apr 12, 2024

The windows build now needs Visual Studio Code 2022.
I updated this in the build instructions.
@brenodt which version are we using on the CI?

@brenodt
Copy link
Collaborator

brenodt commented Apr 15, 2024

Hi @MathJud , the version setup in CI is Visual Studio 2022, as we use the machine executor win/server-2022.

ref: circleci_config/config-continuation/jobs/build-flutter-windows.yml:2

Sources:

@MathJud
Copy link
Member

MathJud commented May 5, 2024

The UI tooltip on the microphone recording icon show the string Send File (coming from sendFileTooltip).
There should be an own tooltip for the microphone recording icon, saying: Record audio message

@sukhman-sukh
Copy link
Author

sukhman-sukh commented May 6, 2024

Hey @MathJud,
I have fixed the UI toolkit for the record audio feature and updated l10n for sendAudioTooltip for all languages and pushed to feat/audio-chat/rebased branch.

@MathJud
Copy link
Member

MathJud commented May 8, 2024

The development of this PR continues in the new PR #628 628

@MathJud MathJud closed this May 8, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Record and Send Voice Messages
3 participants