audio messages #10810
audio messages #10810
Conversation
Jenkins BuildsClick to see older builds (138)
|
1 similar comment
|
i added a testing ui to play with audio formats for interoperability testing and audio quality\size balancing |
mobile/js_files/package.json
Outdated
| @@ -10,6 +10,7 @@ | |||
| "app:android": "react-native run-android" | |||
| }, | |||
| "dependencies": { | |||
| "@react-native-community/audio-toolkit": "git+https://github.com/tbenr/react-native-audio-toolkit.git#v2.0.3-status", | |||
There was a problem hiding this comment.
Why do we need yet another fork of a package?
| @@ -419,6 +421,7 @@ DEPENDENCIES: | |||
| - React-RCTVibration (from `../node_modules/react-native/Libraries/Vibration`) | |||
| - ReactCommon/callinvoker (from `../node_modules/react-native/ReactCommon`) | |||
| - ReactCommon/turbomodule/core (from `../node_modules/react-native/ReactCommon`) | |||
| - "ReactNativeAudioToolkit (from `../node_modules/@react-native-community/audio-toolkit`)" | |||
There was a problem hiding this comment.
Why is this the only one quoted?
There was a problem hiding this comment.
i haven't edit this maually.. i just did make shell -> yarn add ... and make nix-update-pods
i think all @react-native-community modules has "@" so you have all them quoted
There was a problem hiding this comment.
I pinged react-native-community lol.
|
Looks like there's an issue with your dependency: Have you at any point updated the |
I dont think so, we discussed earlier this point so i should had care of it. |
|
I will need to take a look and debug |
|
I think I found the issue: NixOS/nix#2385 According to a commit that closed that issue: But that line isn't there in current version of docs: Which doesn't make much sense to me. I'll try to reproduce. |
|
Yup, tested it with a simple Nix derivation: (import <nixpkgs> {}).stdenv.mkDerivation {
name = "test";
src = builtins.fetchGit {
url = "https://github.com/tbenr/react-native-audio-toolkit.git";
ref = "v2.0.3-status";
};
}And it fails the same way: But it fetches it without issues if I change the And when I checkout the repo the issue is clearly that |
|
I created an issue about it: #10825 |
yes i did. I'll change it if continues failing. Thanks!!! |
1 similar comment
2 similar comments
|
yes not blockers just questions |
|
@churik great! |
|
@tbenr again, amazing work! After that is ready to be merged on my side. |
src/status_im/audio/core.cljs
Outdated
| (on-error {:error (.-err %) :message (.-message %)}) | ||
| (on-seek))))) | ||
|
|
||
| (defn canPlay? [player] |
src/status_im/chat/models/input.cljs
Outdated
| :content-type constants/content-type-audio | ||
| :audio-path audio-path | ||
| :audio-duration-ms duration | ||
| :text "Update to latest version to listen to an audio message here!"}))) |
There was a problem hiding this comment.
probably we need to translate this, not sure
src/status_im/events.cljs
Outdated
| @@ -1237,7 +1242,8 @@ | |||
| (fx/defn on-going-in-background [{:keys [db now]}] | |||
| {:db (-> db | |||
| (dissoc :app-active-since) | |||
| (assoc :app-in-background-since now))}) | |||
| (assoc :app-in-background-since now)) | |||
| :dispatch-n '([:audio-recorder/on-background] [:audio-message/on-background])}) | |||
There was a problem hiding this comment.
@flexsurfer you're concerned because i fire two events or because it is a list?
There was a problem hiding this comment.
last one :) usually vector is used
There was a problem hiding this comment.
don't even remember from where i took that :) i'll change it later
|
not sure if ios fail related to this PR, i would merge it, but let's wait for Jakub |
|
@flexsurfer noticed that lately ios build sometime fails. shortly I'll push your suggestions and let's see if builds. |
|
@tbenr @flexsurfer should I retest briefly? |
|
@flexsurfer fixed all. |
|
we could run e2e just to make sure, no manual testing needed |
|
I need you @cammellos to resolve the status-go conflict :-) |
|
@tbenr you can keep the version in this branch (v0.56.1) as it includes the previous changes |
97% of end-end tests have passedFailed tests (3)Click to expand
Passed tests (95)Click to expand |
Is a bug in status-go, not related to this PR and will be fixed in a separate PR status-im/status-go#2006 |
|
other failures are not related to this PR. Ready to merge @cammellos |
cammellos
force-pushed
the
audio-messages
branch
from
6a29c70
to
86700f2
Compare
Signed-off-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>
|
Fantastic work! @tbenr |
fixes #10669
Audio message implementation started
Uses a fork of
react-native-audio-toolkithttps://github.com/tbenr/react-native-audio-toolkit/tree/v2.0.3-status-v6 which implementsdata:audio/mp3;base64,UklGRhwMAABXQVZF...)recording params are: aac, 22050khz, mono, 32kbs
activity tracking:
sender side:
receiver side (chat view)
status: ready