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

[GSoC'21] Livestreaming v2 #387

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

Sov-trotter
Copy link
Member

@Sov-trotter Sov-trotter commented Aug 7, 2021

PR Checklist

If you are contributing to Javis.jl, please make sure you are able to check off each item on this list:

  • Did I update CHANGELOG.md with whatever changes/features I added with this PR?
  • Did I make sure to only change the part of the file where I introduced a new change/feature?
  • Did I cover all corner cases to be close to 100% test coverage (if applicable)?
  • Did I properly add Javis dependencies to the Project.toml + set an upper bound of the dependency (if applicable)?
  • Did I properly add test dependencies to the test directory (if applicable)?
  • Did I check relevant tutorials that may be affected by changes in this PR?
  • Did I clearly articulate why this PR was made the way it was and how it was made?

Closes #345
As discussed this makes livestreaming more live, this time by streaming the frames rather than a gif.
Also allows to stream specific frames by passing in a UnitRange

....

conf = setup_stream(:local, 10:12)
render(video; streamconfig = conf)

@Sov-trotter Sov-trotter changed the title Livestreaming v2 [GSoC'21] Livestreaming v2 Aug 8, 2021
@codecov
Copy link

codecov bot commented Aug 8, 2021

Codecov Report

Merging #387 (4a1b580) into master (2f9e047) will increase coverage by 0.01%.
The diff coverage is 92.85%.

❗ Current head 4a1b580 differs from pull request most recent head f4290ce. Consider uploading reports for the commit f4290ce to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #387      +/-   ##
==========================================
+ Coverage   96.25%   96.26%   +0.01%     
==========================================
  Files          35       35              
  Lines        1521     1528       +7     
==========================================
+ Hits         1464     1471       +7     
  Misses         57       57              
Impacted Files Coverage Δ
src/structs/Livestream.jl 100.00% <ø> (ø)
src/javis_viewer.jl 91.83% <90.90%> (+0.62%) ⬆️
src/Javis.jl 96.39% <100.00%> (+0.03%) ⬆️
src/util.jl 97.02% <0.00%> (-0.06%) ⬇️
src/shorthands/JBox.jl 100.00% <0.00%> (ø)
src/action_animations.jl 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f9e047...f4290ce. Read the comment docs.

@Sov-trotter
Copy link
Member Author

@TheCedarPrince @Wikunia I think this is ready for review

Copy link
Member

@Wikunia Wikunia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest I expected something quite different with #345
Currently this does still precomputes all image frames before starting to stream.
Live streaming should mean that it starts streaming as soon as the first frame got computed.

@@ -1,6 +1,8 @@
# Javis.jl - Changelog

## Unreleased
- Make livestreaming faster
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean by faster? Is it live? 😉

src/Javis.jl Outdated Show resolved Hide resolved
stream_filecounter = 1
for frame in stream_frames
frame_image = convert.(RGB, get_javis_frame(video, objects, frame; layers = layers))
if !isempty(tempdirectory)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if tempdirectory is empty? It just generates the frames but can't use it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah what I mean is: it should never be empty so we don't need the if. The problem is if we have an mp4 as the pathname without a temp directory but want to stream it will fail, right?

@Wikunia
Copy link
Member

Wikunia commented Aug 9, 2021

I'm unsure what broke when merging it with master. Do you have an idea @Sov-trotter ? Feel free to revert the changes I made.

@Sov-trotter
Copy link
Member Author

Live streaming should mean that it starts streaming as soon as the first frame got computed.

Yeah. I understand. Just taking small steps here. Will get to that too. :)

@Sov-trotter
Copy link
Member Author

Live streaming should mean that it starts streaming as soon as the first frame got computed.

I think we cna talk about how to do this in a cleaner fashion. Because subsequent frames are generated in a serial fashion in a for loop.

I wonder how to have ffmpeg read that. We can obviously overwrite one image again and again but that might cause a race condition.

https://newbedev.com/can-you-stream-images-to-ffmpeg-to-construct-a-video-instead-of-saving-them-to-disk might be of help

@Sov-trotter
Copy link
Member Author

Sov-trotter commented Aug 11, 2021

Hey @Wikunia @TheCedarPrince after some more experimentation here's what I found:

  • I think it's more reasonable to have the the the livestreaming "semi-live" as currently in this PR or lazy streaming? Not sure if I am using the term "lazy" correctly?

  • As I mentioned, there are two ways of making it actually live first is using image2pipe and other is streaming a single image and overwriting it in julia.

  • image2pipe works. One work around I found was since it doesn't support -stream_loop we can create our own loop in julia that iterates to large values that creates a new ffmpeg process everytime.

  • This works but is a bit laggy on ffplay, while obs works smoothly.

  • However I find this to be slow since get_javis_frame is called for every iteration of the stream. Hence using image2pipe here is not a good idea since that will require us to pipe the frames for every iteration hence making it computationally expensive.

  • The above point also applies to overwriting the frame image, since we expected some caching of the frames to take place but that doesn't quite happen without using stream_loop.

  • Also creating a new image and replacing the previous frame with the new one isn't neat?

What we can do is remove the images from temp directory when cancel_stream is called.

@Wikunia Wikunia changed the base branch from master to main February 25, 2022 18:37
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.

Make livestreaming live
2 participants