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

Video doesn't autoplay if shared to Newpipe #4071

Closed
opusforlife2 opened this issue Aug 4, 2020 · 13 comments · Fixed by #4259
Closed

Video doesn't autoplay if shared to Newpipe #4071

opusforlife2 opened this issue Aug 4, 2020 · 13 comments · Fixed by #4259
Labels
bug Issue is related to a bug

Comments

@opusforlife2
Copy link
Collaborator

opusforlife2 commented Aug 4, 2020

Version

  • dev branch after 2907

Steps to reproduce the bug

  1. Turn on Autoplay (on Wi-Fi or Always).
  2. Swipe Newpipe away from Recents.
  3. Share a link to Newpipe.

Expected behavior

Video details open up and video starts playing automatically.

Actual behaviour

Video details open up, but that's it. That's the same behaviour as if autoplay was off.

Found while investigating #2232.

@opusforlife2 opusforlife2 added the bug Issue is related to a bug label Aug 4, 2020
@ghost
Copy link

ghost commented Aug 4, 2020

Do you mean the video should start automatically orr autoplay doesn't work properly. That title is messy. Autoplay is when the video has ended and the next video is about to play...

@opusforlife2
Copy link
Collaborator Author

@ingingin You're thinking of auto-queuing of the next stream, which is an option in settings and also a toggle on the video details page. It's incorrectly named, especially now that it conflicts with the new setting added by the unified player PR.

@opusforlife2
Copy link
Collaborator Author

I think the toggle should be renamed to "Auto-play Next" or "Auto-play Next Stream". What do you think, @B0pol, @Stypox?

@avently
Copy link
Contributor

avently commented Sep 3, 2020

Actually the issue is a little bit wrong and here is why. There are couple of choices you can use right now when you share a video to NewPipe: Show Info, Video player, Popup Player, Background Player, Download.
First option (show info) just do what it should do: it just shows the info. Auto play is not working here which is expected.
Video player option will autoplay the video if auto play is enabled globally in the app which is expected too.
Popup and background player will always play automatically which is how they should work too.

In order to enable auto play for show info this line should be added before this line: https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/RouterActivity.java#L459

intent.putExtra(VideoDetailFragment.AUTO_PLAY, true);

But it's not logical choice at all based on how options are named.

@opusforlife2
Copy link
Collaborator Author

Well, yeah. Show info is incorrect if autoplay is on. I think the visibility of the Show Info option should change based on if autoplay is on or off.

With autoplay off it can continue to be:

Show Info, Video player, Popup Player, Background Player, Download.

But with autoplay on it should be:

Video player, Popup Player, Background Player, Download.

@opusforlife2
Copy link
Collaborator Author

opusforlife2 commented Sep 3, 2020

Ok, wait, I just tested with the Always Ask option in the latest debug apk. Both Show Info and Video Player have the exact same behaviour for autoplay off: they open the video details but don't play it. For autoplay on, both open video details, and the video additionally plays if Video Player was selected.

Now, I think a better solution than my comment above is to tweak the behaviour in this way:

  • if autoplay is off, the app should show Show Info as an option, but hide Video Player. (or grey it out)
  • if autoplay is on, the app should show Video Player as an option, but hide Show Info. (or grey it out)

This ^ is for the Always Ask dialog when opened externally, and for the Preferred open action dialog in Settings. Moreover if Video Player is selected as the preferred open action, then turning autoplay off should replace it with Show Info, but keep that radio button selected, and vice versa.

I think this set of behaviours will be the most intuitive.

@avently
Copy link
Contributor

avently commented Sep 3, 2020

@opusforlife2 sounds good actually. Make this happen:)

@opusforlife2
Copy link
Collaborator Author

You hurt me right in my nascent coding skills.

@TobiGr
Copy link
Member

TobiGr commented Sep 9, 2020

  • if autoplay is off, the app should show Show Info as an option, but hide Video Player. (or grey it out)
  • if autoplay is on, the app should show Video Player as an option, but hide Show Info. (or grey it out)

Do you mean the auto-play toggle in terms of auto-queue or auto-play in terms of play video automatically when detail page oepened?
I'd prefer to check for the second one.

@opusforlife2
Copy link
Collaborator Author

The second one. I normally refer to auto-queue as auto-queue only.

@opusforlife2
Copy link
Collaborator Author

Also, auto-queue is not related to this at all. 😆

@avently
Copy link
Contributor

avently commented Sep 10, 2020

This issue should be closed once #4259 be merged, am I right? Or you see another ways to do some things?

@opusforlife2
Copy link
Collaborator Author

Apparently, we blindsided TobiGr with this because #4070 was about the same problem. 🤭 And it was your investigation which showed this was the same issue.

I don't know what he intends to do with this for now. But yes, that PR will close both this and 4070 if it is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is related to a bug
Projects
None yet
3 participants