search

AlfredoSequeida / fvid

fvid is a project that aims to encode any file as a video using 1-bit color images to survive compression algorithms for data retrieval.
MIT License
354 stars 43 forks source link

Fix 1/2 #8 added rate flag. Required macos #19

Closed dtaivpp closed 4 years ago

dtaivpp commented 4 years ago

@AlfredoSequeida this is fix 1 out of 2 needed for this bug.

ffmpeg requires the -r flag to be set for the output file to work correctly. Here is the command I based this change off of.

ffmpeg -framerate 1 -pattern_type glob -i 'fvid_frames/encoded_frames*.png' -r 30 -vcodec libx264rgb out.mp4

This produces the correct output (as far as I can tell) for the video. There is still an issue with the decoding (Lana only gets 50% decoded sadly). Going to bed will look more tomorrow.

AlfredoSequeida commented 4 years ago

@dtaivpp Awesome work! I actually had no idea macOS would require any ffmpeg differences. As soon as I have some time I will try test this!

dtaivpp commented 4 years ago

@AlfredoSequeida Sounds good mate. Still working on the decoding issues. Idk what is going on there.

@Theelgirl you mentioned having Lena blow up to 8 mb video. I'm having that happen as well. Did you find a fix for that?

zavok commented 4 years ago

@dtaivpp I didn't look at your code really close, but I suspect this is due to ffmpeg's input and output frame rates not matching. At least that was the problem when I was tinkering with my own version, ffmpeg would ignore or duplicate some source images depending on flag misconfiguration.

See this line: https://github.com/dtaivpp/fvid/blob/03a0adccc248ed406460b5eb12196dea104e84b5/fvid/fvid.py#L188 'r=30' should probably be 'r=framerate'

AlfredoSequeida commented 4 years ago

@zavok so I tried your solution and it does not seem to work, so I think I need to do more research on how ffmpeg works. Even if the forced frame rate "r" is set to the same as the as "framerate", it does not work on macOS so I am thinking it might be something else.

Theelx commented 4 years ago

@AlfredoSequeida This problem is fixed in the crypto/password PR also, so if that's merged, this won't matter

Theelx commented 4 years ago

Should be fixed with the cython/password commit, closing now.