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

Add in details view video playing. #551

Closed
wants to merge 1 commit into from
Closed

Add in details view video playing. #551

wants to merge 1 commit into from

Conversation

SkyzohKey
Copy link

@SkyzohKey SkyzohKey commented May 11, 2017

Ok well, this is not perfect but it should give a base for further modifications.
I searched the net to embed an Activity inside a Fragment, or a Fragment into
another Fragment but didn't found how. So i just made some copy/paste + code
modifications to enable in details view Video playing, avoiding to open another
activity. This can be enabled via the settings (disabled by default) and is an
experimental stuff, probably not meant to be merged, but at least, it works!

Hope something more competent than me in Android development (cc @theScrabi) will
take a look at this and found a way to make the code smarter.

This PR tries to address the issue #528 :p

There are still issues:

  • I'm not able to theme the MediaController without changing completly the view Theme.
  • The video player is missing buttons for fullscreen, changing orientation and playing in loop.
  • Code is an evil shit and copies a lot of code (don't repeat yourself, xD) from PlayerVideoActivity.java

My best hope is to convert the PlayerVideoActivity as a reusable View that we could just load with some arguments instead of all that garbage code. c:

Enjoy!

Debug APK !

Ok well, this is not perfect but it should give a base for further modifications.
I searched the net to embed an Activity inside a Fragment, or a Fragment into
another Fragment but didn't found how. So i just made some copy/paste + code
modifications to enable in details view Video playing, avoiding to open another
activity. This can be enabled via the settings (disabled by default) and is an
experimental stuff, probably not meant to be merged, but at least, it works!

Hope something more competent than me in Android development (cc @theScrabi) will
take a look at this and found a way to make the code smarter.

This PR tries to address the issue #528 :p
@TobiGr
Copy link
Contributor

TobiGr commented May 18, 2017

While testing the apk you provided I found some issues which might be solved before a merge can be done. But I didn't dig into the code. That's something @theScrabi or @mauriciocolli can do. So maybe you should wait for their response before fixing the issues.

  • On my tablet (800x1280) the video view is not centred when the resolution of the video is lower than the screen resolution. Sometimes it is a little centred but more on the left side than in the middle, sometimes it is completley on the left side of the screen.
    newpipe_details_view_video_playing_1
    If the video's resolution is higher than the screen's, the app gets slow and stops responding and after a while Android asks to close it.
    Maybe the video view should be as width as the screen.

  • An orientation change results in a stopped video. Furthermore the "normal" details view with the video's thumbnail is displayed.

  • Two orientation changes right after another cause a crash of the UI:

java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.MediaController.hide()' on a null object reference 
  at org.schabi.newpipe.fragments.detail.VideoDetailFragment.hideUi(VideoDetailFragment.java:1068) 
  at org.schabi.newpipe.fragments.detail.VideoDetailFragment.access$2500(VideoDetailFragment.java:81) 
  at org.schabi.newpipe.fragments.detail.VideoDetailFragment$10.run(VideoDetailFragment.java:1055) 
  at android.os.Handler.handleCallback(Handler.java:815) 
  at android.os.Handler.dispatchMessage(Handler.java:104) 
  at android.os.Looper.loop(Looper.java:194) 
  at android.app.ActivityThread.main(ActivityThread.java:5576) 
  at java.lang.reflect.Method.invoke(Native Method) at java.lang.reflect.Method.invoke(Method.java:372) 
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:956) 
  at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:751)

At the moment I cannot test it on an other smaller device.

EDIT:
Another crash occurs when the video plays and the up button (back button in the navigation bar) is clicked:

java.lang.NullPointerException: Attempt to invoke virtual method 'void android.widget.MediaController.hide()' on a null object referenceat org.schabi.newpipe.fragments.detail.VideoDetailFragment.hideUi(VideoDetailFragment.java:1068)
at org.schabi.newpipe.fragments.detail.VideoDetailFragment.access$2500(VideoDetailFragment.java:81)
at org.schabi.newpipe.fragments.detail.VideoDetailFragment$10.run(VideoDetailFragment.java:1055)
at android.os.Handler.handleCallback(Handler.java:815)
at android.os.Handler.dispatchMessage(Handler.java:104)
at android.os.Looper.loop(Looper.java:194)
at android.app.ActivityThread.main(ActivityThread.java:5576)
at java.lang.reflect.Method.invoke(Native Method)
at java.lang.reflect.Method.invoke(Method.java:372)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:956)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:751)

The Android hardware back button works as expected and closes the video.

And sorry for posting crash logs.

@mauriciocolli
Copy link
Contributor

He used the slow android's VideoView, and I already told him in the issue (#528) to use the exoplayer.
Also orientation changes have to be considered.

@TobiGr
Copy link
Contributor

TobiGr commented May 18, 2017

Sorry. I missed that.

@SkyzohKey SkyzohKey deleted the feature/details-player branch June 1, 2017 06:11
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.

3 participants