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 streams #210

Merged
merged 7 commits into from
Jul 17, 2014
Merged

Video streams #210

merged 7 commits into from
Jul 17, 2014

Conversation

SebastianSchildt
Copy link
Contributor

These changes allow setting a time for Video Assets. This is useful when switching to live streams which are endless. For example you could change to a surveillance cam for 5 min and then come back to the next asset. Or you can switch to a live TV channel streamed by some server.

The default behaviour is unchanged, when the duration is set to -1. In this case Screenly tries to figure out the length of the asset and put it into the asset DB just as it does now.

What this pull request also does, is accepting rtsp:// URIs, as this is the protocol used by many IP cams.

@vpetersson
Copy link
Contributor

Thank you @SebastianSchildt. We'll review the commit shortly, but at first glance it looks reasonable.

@SebastianSchildt
Copy link
Contributor Author

Ahh I got lost in GIT and forgot a commit.... I just added 54b9a28 to the pull request. It modifies the URL checking in the server. (The previous request only contained the changes to the web interface)

@vpetersson
Copy link
Contributor

Thanks @SebastianSchildt. @brainrape will be looking at this during the day.

@brainrake
Copy link
Contributor

I'd use 0 as the auto-detect value instead of -1, since "Duration: -1 seconds" looks quite weird. I'd also add a hint saying "enter 0 to auto-detect video length", only shown for videos.
This same method can be used with rtmp as well as rtsp so if we're at it, let's add that too.
In addition, file type detection should work with rtsp, eg when you enter an rtsp url the Asset Type remains "Webpage", whereas if you enter a http image or video url, it changes accordingly.

@SebastianSchildt
Copy link
Contributor Author

Yes rtmp should be added, and I guess 0 instead of -1 is ok. I will have access to my development system tomorrow and try to amend the pull request.

Regarding adding a hint to the Webif: I have been thinking about this, but since I am not very fluent with Javascript and not not really understand all the magical frameworks you used for that, I tried to be rather conservative when changing anything :)

@brainrake
Copy link
Contributor

Cool, thanks. I'l add the hint when you're done, and perhaps fix the filetype detection too.

@SebastianSchildt
Copy link
Contributor Author

Ok, everything changed.

  1. Supports rtmp now
  2. Duration=0 means auto detect
  3. When pasting URLs, rtmp or rtsp URLs will make the add asset dialog switch to video

I did not add a hint the web interface, that 0 means auto-detect length for videos. I am too scared of the web frameworks you used and do not know how . If you could do that, I will just pull it back into my repo :D

@brainrake
Copy link
Contributor

Great, thanks! I'll take care of it soon.
On Jul 17, 2014 3:46 PM, "SebastianSchildt" [email protected]
wrote:

Ok, everything changed.

  1. Supports rtmp now
  2. Duration=0 means auto detect
  3. When pasting URLs, rtmp or rtsp URLs will make the add asset dialog
    switch to video

I did not add a hint the web interface, that 0 means auto-detect length
for videos. I am too scared of the web frameworks you used and do not know
how . If you could do that, I will just pull it back into my repo :D


Reply to this email directly or view it on GitHub
#210 (comment).

brainrake added a commit that referenced this pull request Jul 17, 2014
Support rtmp and rtsp video streams, play for fixed amout of time
@brainrake brainrake merged commit 1da9bea into Screenly:master Jul 17, 2014
@brainrake
Copy link
Contributor

Done, feel free to pull and provide any feedback if needed.

@SebastianSchildt SebastianSchildt deleted the video_streams branch July 19, 2014 10:08
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