-
Notifications
You must be signed in to change notification settings - Fork 516
Add volume management to CastPlayer
#2279
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
base: main
Are you sure you want to change the base?
Conversation
c65bd97
to
94ff5e4
Compare
94ff5e4
to
8c5766a
Compare
Very nice change! Thank you very much!
I think this is fine. You are calling
The bug is about setting the device info which is actually done. You can leave that with me. I'm going to send this for internal review now. You may see some more commits being added as I make changes in response to review feedback. Please refrain from pushing any more substantive changes as it will complicate the internal review - thanks! |
+1. Thanks for your PR. Coincidentally I've been working on a similar fix for a while. But I hit the same issue as (seemingly) a bunch of other people out there. E.g.: https://stackoverflow.com/questions/39498516/remotemediaclient-setstreamvolume-doesnt-changing-cast-volume As a result I changed my implementation to not use setStreamVolume, and instead use CastSession#setVolume. I was planning on merging this soon. Did setStreamVolume actually work for you? If so, can you share with me the model of the device that you used to test? FWIW, my plan was to:
|
Hey @AquilesCanta, To test my changes, I did a small change to the demo Cast application similar to the patch below/attached. If you think it is useful to include it, I can make it look a bit nicer and include it in this PR. Basically, I added a slider to change the volume. When sliding ends, I call The Cast device that I used for testing is a Chromecast with Google TV, running Android 14. Note that there's no visual feedback on the TV that the volume is changing. diff --git a/demos/cast/src/main/java/androidx/media3/demo/cast/MainActivity.java b/demos/cast/src/main/java/androidx/media3/demo/cast/MainActivity.java
index ae49a6f45a..ab51c2a87c 100644
--- a/demos/cast/src/main/java/androidx/media3/demo/cast/MainActivity.java
+++ b/demos/cast/src/main/java/androidx/media3/demo/cast/MainActivity.java
@@ -25,6 +25,7 @@ import android.view.View.OnClickListener;
import android.view.ViewGroup;
import android.widget.ArrayAdapter;
import android.widget.ListView;
+import android.widget.SeekBar;
import android.widget.TextView;
import android.widget.Toast;
import androidx.annotation.Nullable;
@@ -85,6 +86,22 @@ public class MainActivity extends AppCompatActivity
playerView = findViewById(R.id.player_view);
playerView.requestFocus();
+ SeekBar volume = findViewById(R.id.volume_bar);
+ volume.setOnSeekBarChangeListener(new SeekBar.OnSeekBarChangeListener() {
+ @Override
+ public void onProgressChanged(SeekBar seekBar, int progress, boolean fromUser) {
+ }
+
+ @Override
+ public void onStartTrackingTouch(SeekBar seekBar) {
+ }
+
+ @Override
+ public void onStopTrackingTouch(SeekBar seekBar) {
+ playerView.getPlayer().setVolume(seekBar.getProgress() / 100f);
+ }
+ });
+
mediaQueueList = findViewById(R.id.sample_list);
ItemTouchHelper helper = new ItemTouchHelper(new RecyclerViewCallback());
helper.attachToRecyclerView(mediaQueueList);
diff --git a/demos/cast/src/main/res/layout/main_activity.xml b/demos/cast/src/main/res/layout/main_activity.xml
index 81320bb75b..5778e6a2e4 100644
--- a/demos/cast/src/main/res/layout/main_activity.xml
+++ b/demos/cast/src/main/res/layout/main_activity.xml
@@ -27,6 +27,12 @@
android:background="@android:color/black"
app:repeat_toggle_modes="all|one"/>
+ <SeekBar android:id="@+id/volume_bar"
+ android:layout_width="match_parent"
+ android:layout_height="48dp"
+ android:min="0"
+ android:max="100"/>
+
<RelativeLayout android:layout_width="match_parent"
android:layout_height="0dp"
android:layout_weight="1">
|
I appreciate the diff but it's not necessary. I already have a similar local setup. What I was interested in learning was about the receiver device. It's strange I also tried with a chromecast running Android TV. Also tried with a Pixel tablet. And in both cases setStreamVolume didn't do anything. Let me dig a bit deeper. |
8fa9d0f
to
c84d527
Compare
Hey @AquilesCanta, I see that you pushed some changes related to volume management in the |
So, even though I haven't managed to get RemoteMediaClient#setStreamVolume to work. I can confirm that CastSession#setVolume behaves as you'd expect for Player#setDeviceVolume. E.g. a UI slider shows up when I cast to a tablet and I call Player#setVolume. So I'm inclined to think RemoteMediaClient (when working properly) maps reasonably well to Player#setVolume. If you'd like to rebase your PR so that you only include the Player#setVolume (and family) method (and leave the #setDeviceVolume bit working as in the main tree) we can merge that part. Also, if you have any input on the solution using CastSession#setVolume, that'd be welcome. E.g.
Using CastSession#setVolume, I'm getting a volume slider. |
Sure, I'll rebase my PR so it only includes |
96cb2ed
to
e81c319
Compare
e81c319
to
5e97093
Compare
@AquilesCanta I've updated my PR to fix the conflicts. It should be good for a second round of review now. |
This PR enables volume management in the
CastPlayer
. In particular, the following methods are now supported:setVolume()
getVolume()
isDeviceMuted()
setDeviceMuted()
I've also updated the
fetchDeviceInfo()
method so now the max volume information is also provided.Note: I didn't do anything for the
setDeviceVolume()
,increaseDeviceVolume()
anddecreaseDeviceVolume()
methods. Let me know if something should be done there too.Note 2: I don't have access to b/364580007 (the TODO I modified in
CastPlayer
). Maybe I'm missing some information or it needs to be updated.