Fast rewind forward in background activity

[TheLastGimbus]

What is it?

• Bug fix (user facing)
• Feature (user facing)
• Code base improvement (dev facing)
• Meta improvement to the project (dev facing)

Description of the changes in your PR

• Add fast rewind / fast forward in BackgroundPlayerActivity (activity where you manage videos playing in background)

Honestly, 9 icons in one row is maybe too much? We could move shuffle and repeat upward. And we could remove (one of) playbackSpeedButton/playbackPitchButton, since they both do the same.

Fixes the following issue(s)

fixes #2722

Agreement

• I carefully read the contribution guidelines and agree to them.

[Stypox]

Only the playbackSpeedButton should be kept, to stay consistent with players.
Maybe you could add only the fast-forward button and put it where playbackPitchButton is currently. This would not create many problems imho, since fast-forwarding is more used than fast-rewinding, and the latter option can be obtained anyway by moving back the progress bar by a bit and then fast-forwarding until the correct point is reached, thus not wasting much time.

[TheLastGimbus]

I would totally use fast rewind all the time. Like when you listen to some long talk, and missed what the guy just said, and you just want to repeat that - not mess with scroll bar. As for fast forward - I would almost never use that.

Listening in background is either for music or some talks - and you totally need fast rewind for talks.

I would lift pich/speed control up - you are probably not going to click that many times.

[Stypox]

I would totally use fast rewind all the time.

I see, sorry for my wrong assumptions ;-)

I would lift pich/speed control up - you are probably not going to click that many times.

What about moving it to the app bar at the top, where there is also the mute button?

[TheLastGimbus]

Progress bar now updates when you press fast forward/rewind and it's paused.
Sorry but android xml layout gives me a headache :/ (in term of moving speed control on right of video title, where it's most intuitive)
But I can try to move it to app bar as you said, but I don't think it will look well/intuitive, and the app bar's title won't fit.
I think we could replace mute button itself. I don't see any use for it in background player - don't know why it was added in first place (?).

[Stypox]

Sorry for the late response. Apparently the mute button is present in the background player because some people find it useful: #3275 #2876 (comment)
But since it pretty much is a niche feature, I would be ok moving it to the three-dot menu (or showing it ifRoom, @Poolitzer @RickyM7 would that hurt much?). Then its place could be taken by the 1x button, which is not that big (i.e. at maximum 5 characters: 1.11x). And the current place of the 100% and 1x would be taken by fast forward and rewind.

[RickyM7]

@Stypox In my opinion, as long as it keeps working as it is now, I don't care if the button is moved, the important thing is to work well.

[Stypox]

@TheLastGimbus could you implement the requested changes, if you have time? ;-)

[TheLastGimbus]

Sorry, some other stuffed poped up (I've fell into home automation rabbit hole :P )
Thanks for reminding, I'll try to do it today <3

[TheLastGimbus]

Okay soo... that's it.
I'm a little worried about the mute button now - as it is hidden in three dots menu - if someone presses it by accident, and then is like "why the hell video is not playing?"
But NewPipe users should be techy enough to find out :P
What do you think?

[wb9688]

Your changes cause NewPipe to crash when you rotate your device in this activity.

[TheLastGimbus]

It does crash...

org.schabi.newpipe.report.ErrorActivity: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.ImageButton.setOnClickListener(android.view.View$OnClickListener)' on a null object reference
        at org.schabi.newpipe.player.ServicePlayerActivity.buildControls(ServicePlayerActivity.java:323)

It's a null object on fastRewindButton.setOnClickListener(this);
But fastRewindButton exitst... just like other buttons... or does it?

[wb9688]

@TheLastGimbus: No, it doesn't, as you didn't make the appropriate changes to the landscape version of the layout: https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/res/layout-land/activity_player_queue_control.xml

[TheLastGimbus]

Whoopsie, sorry. Now it works :)

[TheLastGimbus]

Sorry that all of this took so long. All of those Java findViewById's and setOnClickListener's made the code somewhat confusing to me as a begginer on this project.
Do you maybe plan to migrate it to Kotlin? It would make the code soo much cleaner and easier to maintain.
@Stypox said in #2714:

I am in favour of the introduction of Kotlin, since it is faster to program in and the transition from Java is smooth

and I totally agree

[wb9688]

Sorry that all of this took so long.

No need to apologize.

All of those Java findViewById's and setOnClickListener's made the code somewhat confusing to me as a begginer on this project.
Do you maybe plan to migrate it to Kotlin? It would make the code soo much cleaner and easier to maintain.

That's just Android's API. I don't see how Kotlin would help with that. We're not actively converting the code to Kotlin, but we accept new code written in Kotlin in NewPipe. We're going to fully move to view binding (or data binding when view binding isn't enough for that fragment) though, see #3697. Also, NewPipe's code is currently just pretty crappy.

I've fell into home automation rabbit hole :P

What are you using for that? I'm using Home Assistant and have it connected to my Hue lights, some Sonoff S20 switches, my UniFi APs for presence detection, and some other small stuff.

[TheLastGimbus]

That's just Android's API. I don't see how Kotlin would help with that

Oh, a lot! Kotlin pretty much does that for you. You don't need to do anything like

private ImageButton fastRewindButton;
fastRewindButton = rootView.findViewById(R.id.control_fast_rewind);
fastRewindButton.setOnClickListener(this);
...
// some global method onClick
@Override
public void onClick(final View view) {
    switch(view.getId()) // need to check which is it
}
...
fastRewindButton.setText("Something");

you just do

fastRewindButton.text = "Something"
fastRewindButton.setOnClickLister {
    // stuff
}

and it's done.
But I've also heard that view bindings are recommended now and kida better.
Also no problems with null checking, null exceptions etc. Kotlin really saves a lot of time, space and readability.

What are you using for that? I'm using Home Assistant

Yass, HASS is amazing. I use ESP8266's flashed with ESPHome. It enables me to connect anything in my home, with any sensor I want, and then HASS allows me to bind them with each other to do crazy stuff, almost like my home is alive.
I can have my plants watered when a new person gets coronavirus.
It's mindblowing

[TheLastGimbus]

I just discovered that when I use some other way to click backward/forward - that is, buttons on headphones, or volume buttons on LineageOS - it activates next/previous video instead of fast forward/rewind.
It is counter intuitive, because the notification has only "fast buttons", so it suggests that this is the main option or something.

Spotify in "podcast mode" has those external buttons connected to forward/rewind - could we do the same, or this is intentional?

[wb9688]

You don't need to do anything like

Neither do you on Java:

button.setOnClickListener((View v) -> {
    v.doWhatever();
});

That's also much clearer to me than:

button.setOnClickListener {
    it.doWhatever()
}

View binding also isn't specific to Kotlin, but works on Java as well. Null is also not really a problem if you properly annotate everything.

Btw I also use some ESP8266s flashed with ESPHome for e.g. temperature, though I have some Hue lights as well.