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: add speedup & down to sound player UI #1530

Merged
merged 10 commits into from
Jan 6, 2024

Conversation

MahmoudMabrok
Copy link
Contributor

@MahmoudMabrok MahmoudMabrok commented Jan 8, 2021

implement the feature at #1529

@princeparacha58
Copy link

Assalam mu alikum brother, i need speed up and speed down in quran andriod audio player please add in your software jazakallah

@MahmoudMabrok
Copy link
Contributor Author

@princeparacha58 I think now we have hadr recitation, so if still need this feature, we can have a build from this PR, but I think it will not have all data sources in current app version.

closed as hadr recitation added.

@ahmedre ahmedre reopened this Dec 24, 2023
@ahmedre
Copy link
Contributor

ahmedre commented Dec 24, 2023

let's keep it open, thinking to merge it but with guidelines on what max speed to set depending on the Qari's rate of recitation, since this remains the top requested feature.

@MahmoudMabrok
Copy link
Contributor Author

if will be merged @ahmedre I would spend today and rest of week on working on it.

I really want to contribute and help Quran users.

@ahmedre
Copy link
Contributor

ahmedre commented Dec 24, 2023

planning to merge it in sha' Allah - جزاكم الله خيراً for building it and really sorry for being quiet about it all this time (as you know, I was against this idea from earlier on, but after discussions and brother @mohamede1945's idea to keep things beneficial, and it being the top feature request), planning on seeing this through in sha' Allah.

@MahmoudMabrok
Copy link
Contributor Author

No problem my brother, so I think we can make make 1.5(as max to all recitation) until we get a comprehensive solution to be related to Quran recitation type, do you agree @ahmedre ?

I follow rule said write, revisit, refactor, enhance

@ahmedre
Copy link
Contributor

ahmedre commented Dec 24, 2023

Sure, that sounds reasonable. Cc @mohamede1945 fyi

Mahmoud Maghrabia and others added 2 commits December 24, 2023 13:11
# Conflicts:
#	app/src/main/java/com/quran/labs/androidquran/service/AudioService.java
#	app/src/main/java/com/quran/labs/androidquran/view/AudioStatusBar.java
@MahmoudMabrok
Copy link
Contributor Author

@ahmedre I have pushed the changes. please check

Copy link
Contributor

@ahmedre ahmedre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good, جزاكم الله خيراً

a few minor things:

  1. we probably should have some pre-sets instead (ex 0.5x, 0.75x, 1x, 1.25x, 1.5x) instead of a dynamic increase/decrease.
  2. if this only works on M+, let's not show the ui components for versions less than M

@MahmoudMabrok
Copy link
Contributor Author

@ahmedre these pre sets will be in code only or will reflect in UI.

@ahmedre
Copy link
Contributor

ahmedre commented Dec 24, 2023

we'd need to reflect in the UI also. for the audio bar, we can do the same as repeat (if you tap the repeat icon, the number cycles there between the valid repeat amounts). then, we can also expose a similar layout to repeat in the settings bottom sheet.

Copy link

OLD: app-madani-debug.apk (signature: V1, V2)
NEW: app-madani-debug.apk (signature: V1, V2)

          │            compressed            │           uncompressed           
          ├───────────┬───────────┬──────────┼───────────┬───────────┬──────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff     
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
      dex │  12.6 MiB │  12.6 MiB │   +592 B │    32 MiB │    32 MiB │ +1.7 KiB 
     arsc │   2.2 MiB │   2.2 MiB │   +292 B │   2.2 MiB │   2.2 MiB │   +292 B 
 manifest │   5.6 KiB │   5.6 KiB │     -1 B │  27.1 KiB │  27.1 KiB │      0 B 
      res │   1.6 MiB │   1.6 MiB │ +1,002 B │   1.8 MiB │   1.8 MiB │ +1.3 KiB 
    asset │ 404.2 KiB │ 404.2 KiB │      0 B │ 678.6 KiB │ 678.6 KiB │      0 B 
    other │   189 KiB │ 189.2 KiB │   +176 B │ 387.1 KiB │ 387.5 KiB │   +402 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
    total │    17 MiB │    17 MiB │   +2 KiB │  37.1 MiB │  37.1 MiB │ +3.6 KiB 


@mohamede1945
Copy link

Sure, that sounds reasonable. Cc @mohamede1945 fyi

Sounds great. Jazak Allah Khyran. And thanks brother @MahmoudMabrok for implementing it.

BTW, Here is the list that the quran.com website shows. I think this PR will be just for [0.5, 0.75, 1, 1.25, 1.5], right?

image

@MahmoudMabrok
Copy link
Contributor Author

@ahmedre
@mohamede1945

as agreed I added them as pre-sets.
I had to move text above icon into left as space was small [check final screenshot]
Screenshot_20231224_215434

Copy link

OLD: app-madani-debug.apk (signature: V1, V2)
NEW: app-madani-debug.apk (signature: V1, V2)

          │            compressed            │           uncompressed           
          ├───────────┬───────────┬──────────┼───────────┬───────────┬──────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff     
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
      dex │  12.6 MiB │  12.6 MiB │ +1.1 KiB │    32 MiB │    32 MiB │ +2.3 KiB 
     arsc │   2.2 MiB │   2.2 MiB │   +140 B │   2.2 MiB │   2.2 MiB │   +140 B 
 manifest │   5.6 KiB │   5.6 KiB │     -1 B │  27.1 KiB │  27.1 KiB │      0 B 
      res │   1.6 MiB │   1.6 MiB │   +507 B │   1.8 MiB │   1.8 MiB │   +704 B 
    asset │ 404.2 KiB │ 404.2 KiB │      0 B │ 678.6 KiB │ 678.6 KiB │      0 B 
    other │   189 KiB │ 189.1 KiB │   +133 B │ 387.1 KiB │ 387.3 KiB │   +194 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
    total │    17 MiB │    17 MiB │ +1.8 KiB │  37.1 MiB │  37.1 MiB │ +3.3 KiB 


@ahmedre
Copy link
Contributor

ahmedre commented Dec 25, 2023

جزاكم الله خيراً
Did some minor clean up - it's still not ready yet, need to figure out a fix for sura transitions where the speed delta gets lost when one sura ends and another starts to play. Likely need to put the speed inside the AudioRequest itself. Will look over the weekend or next week in sha' Allah.

Copy link

OLD: app-madani-debug.apk (signature: V1, V2)
NEW: app-madani-debug.apk (signature: V1, V2)

          │            compressed            │          uncompressed          
          ├───────────┬───────────┬──────────┼───────────┬───────────┬────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff   
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼────────
      dex │  12.6 MiB │  12.6 MiB │   +900 B │    32 MiB │    32 MiB │ +2 KiB 
     arsc │   2.2 MiB │   2.2 MiB │   +140 B │   2.2 MiB │   2.2 MiB │ +140 B 
 manifest │   5.6 KiB │   5.6 KiB │     -1 B │  27.1 KiB │  27.1 KiB │    0 B 
      res │   1.6 MiB │   1.6 MiB │   +508 B │   1.8 MiB │   1.8 MiB │ +704 B 
    asset │ 404.2 KiB │ 404.2 KiB │      0 B │ 678.6 KiB │ 678.6 KiB │    0 B 
    other │   189 KiB │ 189.1 KiB │   +125 B │ 387.1 KiB │ 387.3 KiB │ +194 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼────────
    total │    17 MiB │    17 MiB │ +1.6 KiB │  37.1 MiB │  37.1 MiB │ +3 KiB 


@MahmoudMabrok
Copy link
Contributor Author

I tried to add to AudioRequest but faced a lot of issues.
I will check ISA.

Also add the wiring to allow for exposing a speed setting in the audio
playback panel.
@ahmedre
Copy link
Contributor

ahmedre commented Jan 6, 2024

@MahmoudMabrok I've fixed some issues (including when we transition suras, the playback speed is now properly maintained). I still see two things before we can merge this:

  1. the bigger problem - repeat seek timings are off when repeating at a higher play rate - ex play and press the button to repeat the ayah (and make speed 1.5x) - you'll notice that it reads the ayah and some of the next before going back (at 1x it's fine). note that "seek" itself is fine - i.e. going to the next or previous ayah goes to the correct time stamp.
  2. nice to have - expose this in the audio panel settings - I've added the wiring, just needs the ui piece.

I think the first is caused by our audio check being at 1x - we need to run this more (or less) frequently to fix. the second one just needs to be done. just updating you so you know.

Copy link

github-actions bot commented Jan 6, 2024

OLD: app-madani-debug.apk (signature: V1, V2)
NEW: app-madani-debug.apk (signature: V1, V2)

          │            compressed            │           uncompressed           
          ├───────────┬───────────┬──────────┼───────────┬───────────┬──────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff     
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
      dex │  12.6 MiB │  12.6 MiB │ +1.3 KiB │    32 MiB │    32 MiB │ +2.9 KiB 
     arsc │   2.2 MiB │   2.2 MiB │   +140 B │   2.2 MiB │   2.2 MiB │   +140 B 
 manifest │   5.6 KiB │   5.6 KiB │     -1 B │  27.1 KiB │  27.1 KiB │      0 B 
      res │   1.6 MiB │   1.6 MiB │   +508 B │   1.8 MiB │   1.8 MiB │   +704 B 
    asset │ 404.2 KiB │ 404.2 KiB │      0 B │ 678.6 KiB │ 678.6 KiB │      0 B 
    other │   189 KiB │ 189.1 KiB │   +121 B │ 387.1 KiB │ 387.3 KiB │   +194 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
    total │    17 MiB │    17 MiB │ +2.1 KiB │  37.1 MiB │  37.1 MiB │ +3.9 KiB 


The audio service continuously sends events checking the playback status
every few seconds, depending on how much time is left until the end of
the ayah. These update timings need to now factor in the speed, so if
we are 3 seconds away from the end of the ayah and are playing at 1.5x,
for example, checking after 3 seconds is too late (since we would have
played some of the next ayah).
Copy link

github-actions bot commented Jan 6, 2024

OLD: app-madani-debug.apk (signature: V1, V2)
NEW: app-madani-debug.apk (signature: V1, V2)

          │            compressed            │           uncompressed           
          ├───────────┬───────────┬──────────┼───────────┬───────────┬──────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff     
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
      dex │  12.6 MiB │  12.6 MiB │ +2.5 KiB │    32 MiB │    32 MiB │ +5.3 KiB 
     arsc │   2.2 MiB │   2.2 MiB │   +712 B │   2.2 MiB │   2.2 MiB │   +708 B 
 manifest │   5.6 KiB │   5.6 KiB │     -1 B │  27.1 KiB │  27.1 KiB │      0 B 
      res │   1.6 MiB │   1.6 MiB │ +1.3 KiB │   1.8 MiB │   1.8 MiB │ +2.5 KiB 
    asset │ 404.2 KiB │ 404.2 KiB │      0 B │ 678.6 KiB │ 678.6 KiB │      0 B 
    other │   189 KiB │ 189.2 KiB │   +190 B │ 387.1 KiB │ 387.5 KiB │   +396 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
    total │    17 MiB │    17 MiB │ +4.7 KiB │  37.1 MiB │  37.1 MiB │ +8.9 KiB 


Copy link

github-actions bot commented Jan 6, 2024

OLD: app-madani-debug.apk (signature: V1, V2)
NEW: app-madani-debug.apk (signature: V1, V2)

          │            compressed            │           uncompressed            
          ├───────────┬───────────┬──────────┼───────────┬───────────┬───────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff      
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼───────────
      dex │  12.6 MiB │  12.6 MiB │ +2.5 KiB │    32 MiB │    32 MiB │  +5.3 KiB 
     arsc │   2.2 MiB │   2.2 MiB │   +2 KiB │   2.2 MiB │   2.2 MiB │    +2 KiB 
 manifest │   5.6 KiB │   5.6 KiB │     -1 B │  27.1 KiB │  27.1 KiB │       0 B 
      res │   1.6 MiB │   1.6 MiB │ +1.3 KiB │   1.8 MiB │   1.8 MiB │  +2.5 KiB 
    asset │ 404.2 KiB │ 404.2 KiB │      0 B │ 678.6 KiB │ 678.6 KiB │       0 B 
    other │   189 KiB │ 189.2 KiB │   +178 B │ 387.1 KiB │ 387.5 KiB │    +396 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼───────────
    total │    17 MiB │    17 MiB │   +6 KiB │  37.1 MiB │  37.1 MiB │ +10.2 KiB 


@ahmedre
Copy link
Contributor

ahmedre commented Jan 6, 2024

aforementioned changes made al7amdulillah.
جزاكم الله خيراً @MahmoudMabrok

@ahmedre ahmedre merged commit 77223ad into quran:master Jan 6, 2024
4 checks passed
@MahmoudMabrok
Copy link
Contributor Author

Thanks so much for your efforts and merge.

@MahmoudMabrok MahmoudMabrok deleted the add_sound_up_down branch January 6, 2024 20:16
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.

4 participants