Conversation
Faster livestreaming; stream desired frames
fix tests
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
Remove unused dispatch
|
@TheCedarPrince @Wikunia I think this is ready for review |
Wikunia
left a comment
There was a problem hiding this comment.
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.
| # Javis.jl - Changelog | ||
|
|
||
| ## Unreleased | ||
| - Make livestreaming faster |
There was a problem hiding this comment.
what do you mean by faster? Is it live? 😉
src/javis_viewer.jl
Outdated
| stream_filecounter = 1 | ||
| for frame in stream_frames | ||
| frame_image = convert.(RGB, get_javis_frame(video, objects, frame; layers = layers)) | ||
| if !isempty(tempdirectory) |
There was a problem hiding this comment.
what happens if tempdirectory is empty? It just generates the frames but can't use it?
There was a problem hiding this comment.
No tempdir is created before if it's empty (as before)
https://github.com/Wikunia/Javis.jl/pull/387/files/56156371a5657544c632a3cb6faa2b39681e1fe7#diff-763ffddd0fe1ffb533564007035f5947812905676655f155ce1ee4357eb53c5eR275
There was a problem hiding this comment.
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?
|
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. |
Yeah. I understand. Just taking small steps here. Will get to that too. :) |
MInor fix
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 |
|
Hey @Wikunia @TheCedarPrince after some more experimentation here's what I found:
What we can do is remove the images from temp directory when |
pipe - cache and stream
PR Checklist
If you are contributing to
Javis.jl, please make sure you are able to check off each item on this list:CHANGELOG.mdwith whatever changes/features I added with this PR?Project.toml+ set an upper bound of the dependency (if applicable)?testdirectory (if applicable)?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