Playback inconsistencies about toro HOT 52 CLOSED

Ha, got it.

from toro.

Hi @brittonjg . Thanks for your detailed issue. I would like to response each point :D

1. Yeah ViewPager is a pain. I'm thinking of using GlobalFocusChangedListener to catch the "in-screen" RecyclerView. There is 2 points here: first - it is said that the ViewTreeObserver retrieved from view.getViewTreeObserver() is not always alive. second - Google's Youtube Player API use the same thing in between the onAttachToWindow and onDetachFromWindow. So I think this could solve the problem.
2. This is maybe my fault. I will investigate.
3. I want to research a little bit more. But in the meantime, it seems that loop playback is highly requested. I would try to do something in next release soon.

Hopefully 1.2.0 will come with the support for ViewPager's page changes and loop playback. So stay tune :D. Thanks for using this again. And please feel free to report whatever issue you have.

from toro.

@eneim I only have the utmost respect for your library. I have tried my own implementations, other peoples and combinations of the above. This is just not a simple task, so I thank you for your work here!

Excellent news! Are you looking at days, weeks or longer for that release? I can always do some more investigation if it would help? The looping is definitely a must for me! 👍 Although it would be good to have some way of catching understanding when a loop has occurred, with a callback or something similar.

from toro.

@eneim I have noticed that once a video plays, it is then triggered to play again when the user scrolls the list (as long as the bounds are still valid). This sometimes lags or doesn't render correctly.

Is the restarting of the content, the intended behavior?

Forgot to include, if I want to implement my own ToroViewHolder. Shouldn't the helper VideoViewHolderHelperbe accessible to me? It won't work without it, but I don't seem to have access to it. Your sample works because ToroViewHolder is within the same package.

from toro.

@brittonjg Thanks. For the later comment, I think I noticed it too and made a commit. I will release next one within 1 week or so, not too long :D. I'm reminding some minor fixes too, so please wait.

from toro.

hi @brittonjg , could you use this to test with your current issues?

compile 'com.github.eneim:Toro:develop-SNAPSHOT'

Except for the ViewPager issue, I tried to give a try on loop playback and some other tune up. Please take a look.

from toro.

@eneim How to use the new Loop playback feature in 1.2.0?

from toro.

@gouravd I have only just started looking at the changes, but I think it should be something like the below.

from toro.

Forgot to tell. I updated the draft for 1.2.0 on CHANGELOG.md. Please take a look.

from toro.

@gouravd That seems to be working correctly for me. What is your issue?

@eneim Things I have noticed:

1. VideoViewHolderHelper - This is still private, so from what I can tell it's impossible to implement ToroPlayer directly.
2. Looping - Seems to be working great! It would still be good to have a callback to know when it has looped. onPlaybackStopped doesn't get fired and the position never quite reaches the duration in onPlaybackProgress
3. I still seem to have videos that aren't visible playing. This is without changing fragment. I'll try to investigate why this is happening.

from toro.

@brittonjg As said, ViewPager is a little bit more complicated and I didn't have good support for that. So your behavior is unchanged. For the 1st point, it maybe my fault again (It suppose to be public I guess). 2nd point by intended at the point I add the code, and would be improved in the future. Thanks.

@brittonjg Did a look back. I seemed that I marked VideoViewHolderHelper as Internal API by some reason. I would figure out from my note and implementation.

from toro.

@eneim Your support is greatly appreciated.

Yes, setting it to public should be a good fix. Apart from those issues it is coming along nicely

from toro.

@eneim I tried setting an setOnCompletionListener that broke the looping, so I do not think that is quite the way to solve knowing when the looping occurs.

from toro.

@brittonjg Thanks again. Here is my latest fix on loop: fix a line to trigger onPlaybackStopped, then lookup the loop ability: code.

So you mean that call setOnCompletionListener by yourself will broke the loop? That might because you didn't have the VideoViewHelper to get access back to root. Please give a sample code if you don't might :D.

from toro.

@eneim I've got the code checked out, so I will build it from my machine and add the library in manually for now to save you having to upload. I'll check that change now!

from toro.

@eneim Is it best to discuss everything on here or shall we move to another messaging system?

VideoViewHolderHelperand Toro.getHelper need to be public in order for people to implement their own ToroViewHolder.

from toro.

@brittonjg From 1.2.0 I would open a gitter for this kind of discussion though. But for now, it would be Ok.

I will make VideoViewHolderHelper as a public class (not interface anymore. It must be typed) and remove Toro#getHelper() since this method's purpose is not so clear.

from toro.

FYI the Idea after VideoViewHolderHelper is learnt from NestedScrollViewParentHelper, which allows platform to hook in to Client to provide its support.

from toro.

@eneim I have changed those to public in my version and that seems to work well.

I still have videos playing which are off screen for some reason, which seems to happen if I keep my finger held down when scrolling. I'm still trying to investigate this to see if it's my issue, although I'm not sure how it would be.

Edit it only seems to play the next when the currently playing is recycled...

from toro.

@brittonjg

I still have videos playing which are off screen

They are working as before. ViewPager is not supported yet at the mean time. I'm finding a better way.

And VideoViewHolderHelper will be made public so Client could implement their own.

from toro.

@eneim I mean playing off screen vertically/horizontally in the list. Not after swiping left/right. Although it is still in a view pager if that has an effect?

from toro.

Oh sorry I get your point. I focused to much on ViewPager :D. Well could you describe your off-screen issue in a new one and give me some detail: your Layout Manager and orientation, quantity of Videos (one or many), position of first Video (very first one of the list/grid or not). FYI I figured out once before and made a fix on 1.2.0, but let me double check that before the final release.

from toro.

Haha! Not a problem, I suppose it could be ViewPager related, it's awkward enough!

So, I have a recyclerview with "cards" which all have a video, they are all structured with video then text underneath that can be of varying height. Layoutmanager is just a LinearLayoutManager with one column with the orientation locked to portrait. Many videos as I am loading 30 cards at a time.

I'd be very greatful if you could have a look at it. If we can correct these last little bits I would be much happier just going with your library. If you drop me your gmail address I can add you to my internal release of my app. That may give you a better understanding of what I am trying to achieve.

from toro.

Hmm i'm curious about the bug. Please add this: [email protected] .

from toro.

@brittonjg I add some new stuff to develop branch. It gonna be up and running by the time you online. Please checkout compile 'com.github.eneim:Toro:develop-SNAPSHOT' to see if it helps.

from toro.

Opt in link is - https://play.google.com/apps/testing/com.fanatix.mobile

Notice how some videos don't play until they are quite far up the screen. It's because the previous off screen video is still playing. I usually find this by finding one with sound and playing about with the scrolling.

It should be updated on GooglePlay by 7pm GMT with your latest snapshot.

from toro.

Oh thanks. I can use it now. Yeah I could see weird behaviour sometime. I'm suspecting that the CoordinatorLayout takes part in the bug. Could you pull the latest develop branch (with new visibleOffset logic) and log out that offset value on your scroll? I will try my own later.

from toro.

The issue is quite visible in the logs, but the fix might not be so easy to understand. Seems that 1.0 is being returned for items that aren't on the screen. I do have different height cards, but I don't think that would effect this.

VisibleAreaOffset for item0 = 1.0
VisibleAreaOffset for item0 = 1.0
VisibleAreaOffset for item1 = 1.0
VisibleAreaOffset for item0 = 1.0
VisibleAreaOffset for item0 = 1.0
VisibleAreaOffset for item1 = 1.0
VisibleAreaOffset for item0 = 1.0
VisibleAreaOffset for item0 = 1.0
VisibleAreaOffset for item1 = 0.15346535
VisibleAreaOffset for item0 = 0.92574257
VisibleAreaOffset for item0 = 0.92574257
VisibleAreaOffset for item1 = 0.30858085

I am using the latest support libraries. I have had my bets on it being the co-ordinator view for a while. I am tempted to remove it as a test, but that is quite difficult. I will have a look though

from toro.

I removed the CoordinatorLayout and the issue persisted. Really not sure what is causing this now.

For clarification I am using Support libraries 23.2.0 not the latest 24 aplha versions.

from toro.

i see. There is one line above the offset calculation where I think it is the core issue. I will try to debug your view hierarchy to see if my rectangles work well or not :D

from toro.

@eneim I think I have it sorted for 23.2.1 with FastAdapter. I have taken my ViewAnimator out and that seems to have sorted things. At least with my testing this evening. I will work on it further tomorrow to make sure it is working.

Apart from that I just need to sort out the ViewPager stopping the the playback when the user moves to a new tab. Have you managed to get any further with that? I will also look into it tomorrow.

from toro.

@brittonjg Thanks. I'm not really a fan of FastAdapter though, so Don't have many ideas how things work with that. If you have a ViewAnimator wrapped ViewHolder, I'm curious to see how, since It seems to be directly related to the issue.

About ViewPager stuff, there is a high chance I would use Global Focus Change to catch the current active RecyclerView(s). In most difficult case, where there are multiple RecyclerView in one screen, I need a bit more time to figure out a simple way to deal with them though.

from toro.

@brittonjg There is slightly big change I would like to add to current develop SNAPSHOT: instead of asking client to set Toro's config and use it globally, I will add ToroPlayer#isLoopAble() and ask client to return proper boolean value for each ViewHolder. I hope that this way, people could have more control on how and when a Player could be able to loop (for example: too long video should not loop while a short video could).

This comment is to notice you this change (you may use the SNAPSHOT for now though). Final release would come as soon as I figure out how to deal with ViewPager (or not be able to easily deal with it for now :D).

_Update_: I merge the change at this PR

from toro.

@eneim I had the ViewAnimator wrapping the RecyclerView, to transition to a no connection layout. I have now taken that out, but still I get the same issues.

I still get the same issues, although playback does seem to be improving.

Another thing I have noticed is that you always display a "Can't play this video dialog". This happens because I can't return true for the Media Players onError method. Is this something that can be altered?

from toro.

I have noticed that "Can't play this video dialog". Actually, The official VideoView has that, and will show the dialog when your Video is failed to prepared. I would like to keep the custom implement as transparent with it as possible. But I agree that it is so annoying and gonna change it with a better way (a Toast or a callback).

from toro.

I was going to change it to a SnackBar (less intrusive to the user), it is because your onError always returns false I believe. Therefore the VideoView handles the error with its standard process i.e. the dialog message.

  @Override public final boolean onError(MediaPlayer mp, int what, int extra) {
    return mHelper.onError(this, mp, what, extra);
  }

The below should have some way to effect the onError to say that error has been caught and handled appropriately.

@Override public void onPlaybackError(MediaPlayer mp, int what, int extra) {
    mPlayable = false;
  }

At least that is my understanding of how the VideoView works usually. Therefore it would keep with your want for standard functionality whilst still allowing for customisation.

from toro.

I have just released a new version with your latest changes. Unfortunately, it still seems to be susceptible to the scrolling issue.

I shall have a look at the ViewPager issue from my end as well now. As I would like to release it without that issue.

from toro.

I am solely looking into my playback issues today. I am struggling to understand why I have videos playing off screen. Especially when they give me values likes this (the video is now off screen). Is your getRecyclerViewRect meant to be the RecyclerView or the item within it? My itemView is of type CardView, but in your example yours is LinearLayout which seems to be in contrast to the name of that method.

Edit- This is correct as you're looking for the parents getGlobalVisibleRect my search continues.

It would seem, that because I have some text under the video. The code seems to be restarting the video, that is off screen. I am going to try and replicate on your sample app.

Okay, that is easier to replicate than I imagined. Replace the below into your layout vh_toro_video_simple. Now scroll down, so the video is out of video, but the text is in view. It causes the video above the text to restart playing. I was on Deadly Simple Video List with Big Buck Bunny. That seems to cause the issue I am seeing. Now to find out why...

  <TextView
      android:id="@+id/info"
      android:layout_width="match_parent"
      android:layout_height="80dp"
      android:layout_alignParentBottom="true"
      android:layout_alignParentEnd="true"
      android:layout_alignParentRight="true"
      android:layout_below="@id/video"
      android:layout_margin="4dp"
      android:background="#80000000"
      android:gravity="center"
      android:includeFontPadding="false"
      android:paddingBottom="4dp"
      android:paddingEnd="8dp"
      android:paddingStart="8dp"
      android:paddingTop="4dp"
      android:singleLine="true"
      android:textAppearance="@style/TextAppearance.AppCompat.Small"
      android:textColor="@android:color/white"
      />

from toro.

Okay, so something seems pretty strange here. It is definitely an issue within visibleAreaOffset It would seem that when the video goes off screen it causes the code to think that the height is back to full which triggers the method to return 1.0f. I noticed that if the video height between parentRect and videoRect aren't equal when the video is off screen. I am sure something else is incorrect, but my workaround currently is to add parentRect.height() == videoRect.height() to the check.

Interested to know your thoughts on this.

from toro.

@brittonjg I'm reading your log and comments, but _parentRect_ --> the _RecyclerView's rect_ and the videoRect is an Item's video rectangle. So _parentRect_ supposes to be larger than videoRest in general, except for the case your Video is too big.

from toro.

@eneim That confused me as well. I did manage to replicate it on your sample app (as per my comments), so maybe it is worth having a further look into that? I think something strange happens when it isn't just the video in the holder effecting the height.

from toro.

I have 2 screenshots which could be easier to understand:

Note that the selected lines are the VideoView and its rectangle. And you can see its RecyclerView parent and its rect.

• Screenshot 1

from toro.

What happens if you added my change to your TextView? Does it act in the same way on "Deadly Simple Video List"?

from toro.

It'll just be fine. Since I will just care about the VideoView's rectangle.

from toro.

I setup a gitter channel for Toro. Not public yet, but let's have discussion there. Link is here: https://gitter.im/eneim/Toro

from toro.

Has anything changed in the snapshot version regarding the loop? The videos are not playing in loop anymore

from toro.

Stay tune, I just change default behavior to not looping. Please check out latest code base. I'm gonna prepare a release on that soon.

from toro.

The snapshot is not available now (as I am writing this). Could you suggest and alternative

from toro.

I close this issue as 1.2.0 could somehow full-fill the need. And new issue should be open for fresher discussions.

from toro.

What causes cannot resolve Toro library in Android studio

from toro.

@huxaiphaer I have read your email, I'm working on a simple example for that case. Can you bring your topic in a new issue? It will be easier for me to keep track of it. Also, please update Toro to latest version and report any kind of debug log/error log you have. Thanks.

from toro.

from toro.