search

Proryanator / encoder-benchmark

A tool to benchmark your hardware's real-time video encoding capabilities.
GNU General Public License v3.0
63 stars 5 forks source link

Replace from_creation_date in favor of a cross platform solution #23

Closed e-dong closed 9 months ago

e-dong commented 9 months ago

Summary of Changes

Addresses https://github.com/Proryanator/encoder-benchmark/issues/22

Resolver set to 2

See:

Replace FileTime::from_creation_date

See https://github.com/Proryanator/encoder-benchmark/issues/22 for more details.

https://docs.rs/filetime/latest/filetime/struct.FileTime.html#method.from_last_access_time Access time should be avoided. If there was an older ffmpeg log and I ran cat that would update the access time to the current time. That would cause the script to potentially grab an older log. (This would only happen when ffmpeg is done writing the log, and someone accessed an older log)

Future Notes

@Proryanator Perhaps some notes for the future A potential robust solution is to not depend on file metadata to calculate the latest log. Maybe the script could look at the filename of the ffmpeg log. On my linux system it looks like this: ffmpeg-20230916-080718.log. It may be enough to perform a sort descending on the filename and grab the first from the DirEntry vector. Otherwise you can parse the timestamp to create the FileTime and reuse your current implementation to calculate the latest log.

Some race conditions would exist if running the permutor-cli tool multiple times within the same dir. The script would need some sort of unique hash / execution id to ensure it grabs the correct latest log. I suppose you could append that to the ffmpeg log filename.

e-dong commented 9 months ago

@Proryanator Using metadata.created() works for me lol https://doc.rust-lang.org/std/fs/struct.Metadata.html#method.created

I also added regex to match the ffmpeg log files instead filtering through all files within the current directory.

I pushed up what I have for now, I will do a cleanup when I get home later.

e-dong commented 9 months ago

TODO

Proryanator commented 9 months ago

@e-dong I opened another issue though for adding build pipelines for Linux!

I'll test your branch on Windows today.

https://github.com/Proryanator/encoder-benchmark/issues/26

e-dong commented 9 months ago

@Proryanator What do you think of using the tempfile crate for handling temp dirs/files?

Proryanator commented 9 months ago

@Proryanator What do you think of using the tempfile crate for handling temp dirs/files?

Could be a good feature request! I'd say don't worry about adding that into this PR though to keep the scope straight forward.

e-dong commented 9 months ago

@Proryanator What do you think of using the tempfile crate for handling temp dirs/files?

Could be a good feature request! I'd say don't worry about adding that into this PR though to keep the scope straight forward.

Sounds good!

Proryanator commented 9 months ago

I think there's a variable problem (if you see the unit test failure). If you can fix that I can re-run the pipeline!

e-dong commented 9 months ago

I think there's a variable problem (if you see the unit test failure). If you can fix that I can re-run the pipeline!

@Proryanator Whoops nice catch :+1: