Fix autoplaying #136

2017-05-25T19:43:29+02:00

6ea86b96 commented

2017-05-25 19:43:29 +02:00

(Migrated from github.com)

Fixes having to click play twice. We have an autoplay variable but I don't see where it's getting set, so right now no files will actually autoplay still.

Fixes having to click play twice. We have an `autoplay` variable but I don't see where it's getting set, so right now no files will actually autoplay still.

kauffj commented

2017-05-25 22:11:27 +02:00

(Migrated from github.com)

The logic is more confusing than it ought to be, partially due to poor variable naming, but I believe autoplay is set to true when a user clicks the video cover that appears while a video is loading. This is the only case that autoplaying should happen. So I'm not sure this is the right fix.

The logic is more confusing than it ought to be, partially due to poor variable naming, but I believe `autoplay` is set to true when a user clicks the video cover that appears while a video is loading. This is the only case that autoplaying should happen. So I'm not sure this is the right fix.

6ea86b96 commented

2017-05-26 11:07:26 +02:00

(Migrated from github.com)

ah, well that's much easier then @kauffj. We can just cut out that autoplay stuff and always play the video when the VideoPlayer component is rendered, as it's only rendered when it should be playing anyway. Changes pushed.

ah, well that's much easier then @kauffj. We can just cut out that autoplay stuff and always play the video when the `VideoPlayer` component is rendered, as it's only rendered when it should be playing anyway. Changes pushed.

kauffj commented

2017-05-26 15:39:24 +02:00

(Migrated from github.com)

The video player is now rendered automatically if you have already
downloaded the file. So if you are accessing the show page for a URI you've
downloaded previously, it shouldn't autoplay.

On Fri, May 26, 2017 at 5:07 AM, 6ea86b96 notifications@github.com wrote:

ah, well that's much easier then @kauffj https://github.com/kauffj. We
can just cut out that autoplay stuff and always play the video when the
VideoPlayer component is rendered, as it's only rendered when it should
be playing anyway. Changes pushed.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/lbryio/lbry-app/pull/136#issuecomment-304231825, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAgZVsFN42711YDsLi-rDdLe9BBAVIjQks5r9pZOgaJpZM4NmrJm
.

--

Jeremy Kauffman, Founder, LBRY http://lbry.io/
(267) 210-4292

Build LBRY: get https://lbry.io/get, follow https://twitter.com/lbryio,
like https://facebook.com/lbryio

The video player is now rendered automatically if you have already downloaded the file. So if you are accessing the show page for a URI you've downloaded previously, it shouldn't autoplay. On Fri, May 26, 2017 at 5:07 AM, 6ea86b96 <notifications@github.com> wrote: > ah, well that's much easier then @kauffj <https://github.com/kauffj>. We > can just cut out that autoplay stuff and always play the video when the > VideoPlayer component is rendered, as it's only rendered when it should > be playing anyway. Changes pushed. > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/lbryio/lbry-app/pull/136#issuecomment-304231825>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AAgZVsFN42711YDsLi-rDdLe9BBAVIjQks5r9pZOgaJpZM4NmrJm> > . > -- Jeremy Kauffman, Founder, LBRY <http://lbry.io/> (267) 210-4292 Build LBRY: get <https://lbry.io/get>, follow <https://twitter.com/lbryio>, like <https://facebook.com/lbryio>

6ea86b96 commented

2017-05-28 17:20:08 +02:00

(Migrated from github.com)

The VideoPlayer isn't rendered until the file is downloaded/ready to play and isPlaying is true in the Video component https://github.com/lbryio/lbry-app/blob/master/ui/js/component/video/view.jsx#L115

6ea86b96 commented

2017-05-28 17:20:46 +02:00

(Migrated from github.com)

so this commit does actually work. If you downloaded a file, readyToPlay is true but isLoading and isPlaying are false. If a stream is loading then the fileInfo is still null so readyToPlay is false. I haven't been able to break the player in any testing so far.

6ea86b96 commented

2017-05-29 19:16:38 +02:00

(Migrated from github.com)

ok, so it seems like it's not working when you purchase new content. Will look into a fix.

6ea86b96 commented

2017-05-30 07:20:12 +02:00

(Migrated from github.com)

Turns out that this is a pain to fix. We have 2 affirmPurchase modals. One on the file actions and one on the video player. I wanted to hook into the confirmed method to start the video playing but actually the modal on the file actions is the one being displayed.

Seems like we're either going to need to refactor the modals a bit or move the currently playing file into redux, or both.

Turns out that this is a pain to fix. We have 2 `affirmPurchase` modals. One on the file actions and one on the video player. I wanted to hook into the confirmed method to start the video playing but actually the modal on the file actions is the one being displayed. Seems like we're either going to need to refactor the modals a bit or move the currently playing file into redux, or both.

kauffj commented

2017-05-30 20:23:48 +02:00

(Migrated from github.com)

My thought process here would be to move a list of URIs approved for purchase into redux rather than the currently playing file.

6ea86b96 commented

2017-05-30 21:30:00 +02:00

(Migrated from github.com)

Missed your comment earlier @kauffj. I pushed something that works here, but it's a little hacky. For now I just added an option to pass the modal name into the purchase action so we render the correct one.

I also fixed the file buffering issue. I broke that when I switched players. Turns out that playing downloading file streams isn't so easy. Luckily http://webtorrent.to have a similar issue and have created a lib for that. https://github.com/feross/render-media. Seems like a good idea to use? It picks the correct video playback lib to use depending on the file type, and audio/pdf/etc too. Seems like we could change the Video component to be a Media component later and just allow render-media to render whatever we throw at it?

Missed your comment earlier @kauffj. I pushed something that works here, but it's a little hacky. For now I just added an option to pass the modal name into the purchase action so we render the correct one. I also fixed the file buffering issue. I broke that when I switched players. Turns out that playing downloading file streams isn't so easy. Luckily http://webtorrent.to have a similar issue and have created a lib for that. https://github.com/feross/render-media. Seems like a good idea to use? It picks the correct video playback lib to use depending on the file type, and audio/pdf/etc too. Seems like we could change the `Video` component to be a `Media` component later and just allow `render-media` to render whatever we throw at it?

6ea86b96 commented

2017-05-30 21:32:13 +02:00

(Migrated from github.com)

I like the approved for purchase state list though. That's a much better solution :)

6ea86b96 commented

2017-05-30 21:39:38 +02:00

(Migrated from github.com)

Also, although render-media handles fallback depending on file types and errors on the video element, it doesn't catch errors in the audio. There's something wrong with the Coherence movie for example, where it blows up in the mp4 remuxer. Going to have to find some way to catch those errors.

Also, although `render-media` handles fallback depending on file types and errors on the `video` element, it doesn't catch errors in the audio. There's something wrong with the Coherence movie for example, where it blows up in the mp4 remuxer. Going to have to find some way to catch those errors.

kauffj (Migrated from github.com) approved these changes 2017-05-30 23:17:11 +02:00

ui/js/component/video/view.jsx

					
				@ -147,3 +154,3 @@

				    return (

				      <video controls id="video" ref="video" style={{backgroundImage: "url('" + poster + "')"}} >

				      <video controls ref="video" style={{backgroundImage: "url('" + poster + "')"}} >

				        <source src={downloadPath} type={contentType} />