New Feature - Windows Support

[Joly0]

Hey, just wanted to let you know, that i am working on the windows part of this. I havent forked the project yet, but will do once i am finished. So far everything seems to be working, though starting powershell from the batch file with the arguments and everything correct gave me some headache, aswell as some other things. But i am quite sure, i will be done soon and hopefully have everything working by then.

But i wanted to know 2 things:

1. I havent signed my commit ever, so i dont know, how this works, could you please explain this? And why you want this? It feels like it makes i way harder for people to contribute to this project although its quite cool.
2. I saw you have your license information in every file, should i add something similar to files from me aswell? I would just copy/paste yours but replace your name with mine?

[hollowaykeanho]

Hey, just wanted to let you know, that i am working on the windows part of this. I havent forked the project yet, but will do once i am finished.

Thank you!!! My backlog is quite big at the moment.

So far everything seems to be working, though starting powershell from the batch file with the arguments and everything correct gave me some headache, aswell as some other things. But i am quite sure, i will be done soon and hopefully have everything working by then.

Haha.. take your time. I couldn't setup powershell on my Debian system because MSFT Powershell's telemetry. Can't risk it.

I havent signed my commit ever, so i dont know, how this works, could you please explain this? And why you want this? It feels like it makes i way harder for people to contribute to this project although its quite cool.

I'm not sure how it works on Windows. To allow a signed commit, you need not just git but also gpg with your own key. I did a maintenance docs back in the day here for GPG (on how to create the key and protecting it): https://sites.google.com/view/chewkeanho/guides/gnupg

or you can refer to GitHub docs:

https://docs.github.com/en/authentication/managing-commit-signature-verification/generating-a-new-gpg-key

Then in your local repository's .git/config, you need to add something like this:

[user]
        name = (Holloway) Chew, Kean Ho
        email = kean.ho.chew@zoralab.com
        signingkey = YOUR_PUBLIC_GPG_KEY_ID_HERE

On Linux, we have an agent called gpg-agent that helps maintain a session. Otherwise, you have to unlock your GPG Key for signing with password every time you commit.

On GitHub, please remember to add your public key as guided here:

https://docs.github.com/en/authentication/managing-commit-signature-verification/adding-a-gpg-key-to-your-github-account

Don't worry, this is just a side project. GPG signing is not required but recommended. I suggest you can take the opportunity to explore. Let me know if you need help. =)

I saw you have your license information in every file, should i add something similar to files from me aswell? I would just copy/paste yours but replace your name with mine?

Yeap. Same licenses. The owner is you of course. Something like

Copyrights <YEAR_WHEN_PUBLISHED> Full Name with Comma indicator <Email>

....BSD License notice...

Because your script is a copyrightable item on its own and for preventing IP hijacking, that's why I maintain per file basis. If the PowerShell or Batch script commenting system does not support it, then it's okay not to include. Function first.

[hollowaykeanho]

FYI, I'm flexible in the model's config data file (e.g. https://github.com/hollowaykeanho/Upscaler/blob/main/models/upscayl-realesrgan-x4plus-anime.sh)

It's currently written in POSIX script format but I'm open for adjustment (e.g. convert to a common format) in case the efforts are too much on your side.

[Joly0]

FYI, I'm flexible in the model's config data file (e.g. https://github.com/hollowaykeanho/Upscaler/blob/main/models/upscayl-realesrgan-x4plus-anime.sh)

It's currently written in POSIX script format but I'm open for adjustment (e.g. convert to a common format) in case the efforts are too much on your side.

Nah, i am fine with the format, i can work with that. I am so far finished with the ground work, but i need to adjust the code a bit, refactor, cleanup some stuff and especially add error handling. Thats something i have skipped. But so far everything seems to be working.

Afterwards i will look into commit signing, cant be that hard on windows. And push to my fork, but i will let you know when i am done.

[Joly0]

Ok, i think i am finished. I was even able to add a gpg key and sign the commit. Pls take a look at my form here https://github.com/Joly0/Upscaler

So far i tried to make the scripts work 1:1 like yours, though i have not added a lot of error handling (though the most important parts are there). Merging should be fine, but please take a look at my work first.

So far i was able to handle the exact same commands you used for linux to work with powershell and same for the models/*.sh files, no changes are needed, though i cant confirm this will stay like this in the future if things change massively

[hollowaykeanho]

I reviewed your codes. Here are some reqeusts:

https://github.com/Joly0/Upscaler/blob/main/init/windows.ps1:

Line 4

add your contact channel (e.g. email or github). Examples: https://github.com/ZORALab/Hestia/blob/experimental/hestiaGO/hestiaINTERNAL/hestiaMATH/Float64.go

If you want to use github profile, use a bracket like Copyright (c) 2023, Joly0 (https://github.com/Joly0).

If you want to use an email, use the angle bracket like Copyright (c) 2023, Joly0 <me@email.com>.

Function Naming

Line 56 - use dash instead of underscore. The rest were cosistently using dashes. No rules on _ or - but being consistent is one.

Line 63, 83 - add dash after the 2nd word. Consistencies.

Quick question: is it possible to map the naming to be the same as the posix script? If not it's okay...can work the other way round easily.

Tab Spacing

Line 221, 222 - indent inconsistencies.

Line 86 - 100 - definitely indent inconsistencies.

Line 287-290 - definitely indent inconsistencies.

Line Spacing

Line 68, Line 76, Line 80, Line 112, Line 118, Line 122, Line 133, Line 158, Line 162, Line 173, - give 1 line spacing before for ease of readability. (Looks like I need to work on mine as well. Let's merge yours first).

Line 84 - remove line spacing.

Bracket Spacing

Line 308, 313, 318, 322, 356, 335 - no need extra spacing after the open parenthesis.

Inline braces

Line 192 - expand it like Line 198. Don't hide anything into 1 line. If can, move Line 192 down to Line 198 for clarity.

Spelling

Line 181, Line 177,

FFMPEG

Line 254 - I see you had removed the -loglevel error argument compared to mine's line 456 https://github.com/Joly0/Upscaler/blob/main/init/unix.sh#L456

Is it working differently on Windows? Mine is as follows:

u0:Upscaler$ ffprobe -show_entries stream=pix_fmt -of csv=p=0 tests/image/sample-01.jpeg 
ffprobe version 4.3.6-0+deb11u1 Copyright (c) 2007-2023 the FFmpeg developers
  built with gcc 10 (Debian 10.2.1-6)
  configuration: --prefix=/usr --extra-version=0+deb11u1 --toolchain=hardened --libdir=/usr/lib/x86_64-linux-gnu --incdir=/usr/include/x86_64-linux-gnu --arch=amd64 --enable-gpl --disable-stripping --enable-avresample --disable-filter=resample --enable-gnutls --enable-ladspa --enable-libaom --enable-libass --enable-libbluray --enable-libbs2b --enable-libcaca --enable-libcdio --enable-libcodec2 --enable-libdav1d --enable-libflite --enable-libfontconfig --enable-libfreetype --enable-libfribidi --enable-libgme --enable-libgsm --enable-libjack --enable-libmp3lame --enable-libmysofa --enable-libopenjpeg --enable-libopenmpt --enable-libopus --enable-libpulse --enable-librabbitmq --enable-librsvg --enable-librubberband --enable-libshine --enable-libsnappy --enable-libsoxr --enable-libspeex --enable-libsrt --enable-libssh --enable-libtheora --enable-libtwolame --enable-libvidstab --enable-libvorbis --enable-libvpx --enable-libwavpack --enable-libwebp --enable-libx265 --enable-libxml2 --enable-libxvid --enable-libzmq --enable-libzvbi --enable-lv2 --enable-omx --enable-openal --enable-opencl --enable-opengl --enable-sdl2 --enable-pocketsphinx --enable-libmfx --enable-libdc1394 --enable-libdrm --enable-libiec61883 --enable-chromaprint --enable-frei0r --enable-libx264 --enable-shared
  libavutil      56. 51.100 / 56. 51.100
  libavcodec     58. 91.100 / 58. 91.100
  libavformat    58. 45.100 / 58. 45.100
  libavdevice    58. 10.100 / 58. 10.100
  libavfilter     7. 85.100 /  7. 85.100
  libavresample   4.  0.  0 /  4.  0.  0
  libswscale      5.  7.100 /  5.  7.100
  libswresample   3.  7.100 /  3.  7.100
  libpostproc    55.  7.100 / 55.  7.100
Input #0, image2, from 'tests/image/sample-01.jpeg':
  Duration: 00:00:00.04, start: 0.000000, bitrate: 40901 kb/s
    Stream #0:0: Video: mjpeg (Baseline), yuvj420p(pc, bt470bg/unknown/unknown), 768x768 [SAR 1:1 DAR 1:1], 25 tbr, 25 tbn, 25 tbc
yuvj420p

u0:Upscaler$ ffprobe -loglevel error -show_entries stream=pix_fmt -of csv=p=0 tests/image/sample-01.jpeg 
yuvj420p

With -loglevel error it only cherry-pick the value.

Error Handling

I just checked the critical ffmpeg and ffprobe command checking + the binary programs. Since the checkings are inside, I considered it pass. =)

If you're referring to the pretty printing from _print_status function, take your time.

Test Scripts + Files

There is an internal test scripts + files made available in tests/ directory. See if you can bring it up and run a benchmark. I use that benchmark.sh to do script testing and also capturing benchmark so that end-users can make an informed choice (see: https://github.com/hollowaykeanho/Upscaler#benchmarks).

Once done, I consider your scripts are stable enough for use.

---

Once again, thanks for the efforts and commitments.

Don't need to rebase the above changes (unless you know what you're doing). I'm ok with forward-moving timeline with a new commit stating about new learning.

[Joly0]

Thank your for your feedback. I will look into it and fix it.

I have already tried to run the test files, but not with the benchmark.sh file, rather manually, and so far that did work. But i will try to test it. Unfortunately i am in vacation now and the laptop i am using is quite on the weak side for such tasks 😅

[hollowaykeanho]

No hush. Enjoy your vacation. =)

I'll convert the benchmark.sh file with the polygot script so that it will be available for you as well.

UPDATE: benchmark.cmd is now updated. The windows' side is not tested. You can patch it accordingly. On Linux, it was measured into minutes automatically. Seconds can be too big.

[Joly0]

Ok, i think i got everything fixed now. Just some side-notes:

• I could match the same function names as in the posix script, but those are not using powershell naming conventions, so i sticked to that, also the reason the functions now dont have _ (for example here https://github.com/Joly0/Upscaler/blob/f25b7e86b7b53897f31de5729d5afe5722e21ca0/init/windows.ps1#L56 ). I would like to stick to the naming conventions for that
• Loglevel was indeed a copy/paste issue on my side, should be correct
• Somehow indentation issues appeared after pushing. Locally everything was correct, hopefully now its fine aswell
• I could implement __print_status, but i thought thats overkill and not really necessary, so i stick to Write-Host with -ForegroundColor
• benchmark scripts doesnt work for me, i keep getting "Entered time cannot be accepted."-errors. Is there something wrong with this https://github.com/hollowaykeanho/Upscaler/blob/af46b3e6697db2cda666fb17b2b99f7ff4c7db04/tests/benchmark.cmd#L63 ? So i was unable to test that

Everything else should be right now and can be merged imo

[hollowaykeanho]

I could match the same function names as in the posix script, but those are not using powershell naming conventions, so i sticked to that, also the reason the functions now dont have _ (for example here https://github.com/Joly0/Upscaler/blob/f25b7e86b7b53897f31de5729d5afe5722e21ca0/init/windows.ps1#L56 ). I would like to stick to the naming conventions for that

Ok. No problem. Stick to PowerShell convention. Functionality first.

Loglevel was indeed a copy/paste issue on my side, should be correct

Somehow indentation issues appeared after pushing. Locally everything was correct, hopefully now its fine aswell

Much cleaner now. Patch merged. Thank you. =)

Oh yeah, can you give me your github version email address please? (I'm aware you're not displaying your email for privacy concern). Attributing you as co-author. See: https://github.blog/2018-01-29-commit-together-with-co-authors/

It's the github version, not the personal/primary one. Should be availble under "Settings > Emails > Keep my email addresses private" section.

If you're okay to use your primary email. That will work as well.

I could implement __print_status, but i thought thats overkill and not really necessary, so i stick to Write-Host with -ForegroundColor

Yeah it's a low priority thing (good to have). I'm much more happy to accept your code enabling the project for Windows user. =)

benchmark scripts doesnt work for me, i keep getting "Entered time cannot be accepted."-errors. Is there something wrong with this

I haven't test the Windows part. Please feel free to amend it as required. The process I thought of was:

1. capture start time
2. run the benchmark command
3. capture end time
4. calculate duration in minutes / seconds
5. dump out value

It should be batch script calling that start.cmd command (like the way we want user to call).

UPDATE: I found the bug. I accidentally copy-pasted time command in front of ./start.cmd from the UNIX side. Please re-verify. If it works. A copy of the data on Windows would be perfecto.

[Joly0]

Here is the private mail address github is showing me: 13993216+Joly0@users.noreply.github.com

Regarding the benchmark tests i will try to test them today or tomorrow, maybe i have some free time during vacation stuff to do so without annoying my gf to much 😅 but i will let you know once i am finished. I might be able to spin up a vm on my server to test with an nvidia quadro t600, should be equivalent to a gtx 1650 or 1660 (not sure). That should be faster than my igpu only laptop and i have a friend who has a nvidia 3090ti and could maybe benchmark on that

[hollowaykeanho]

Regarding the benchmark tests i will try to test them today or tomorrow, maybe i have some free time during vacation stuff to do so without annoying my gf to much sweat_smile but i will let you know once i am finished.

Haha.. Enjoy the trip ya.

I might be able to spin up a vm on my server to test with an nvidia quadro t600, should be equivalent to a gtx 1650 or 1660 (not sure).

Don't have to intentionally spin a cloud server and burn through money. The ideal case is conventional laptop or pc can use the AI program without bringing in the expensive big guns.

i have a friend who has a nvidia 3090ti and could maybe benchmark on that

Sure. Same rules: don't burn money because of benchmarks. Tough economy these days.

Here is the private mail address github is showing me: 13993216+Joly0@users.noreply.github.com

I work out the co-authors badge for you on my side.

[Joly0]

Btw, while trying to run the benchmark, i found some issues in the benchmark script and in my script aswell. So i made another commit to adjust that. Looks like its running thought now, though benchmark takes a while, so i wont run the whole thing again now, but it goes through now atleast. And i found some duplicate arguments in your last ffmpeg command aswell, but didnt fix that, because i didnt want to break anything, so i will leave that to you.

[hollowaykeanho]

And i found some duplicate arguments in your last ffmpeg command aswell, but didnt fix that, because i didnt want to break anything, so i will leave that to you.

Line number?

Btw, while trying to run the benchmark, i found some issues in the benchmark script and in my script aswell. So i made another commit to adjust that. Looks like its running thought now, though benchmark takes a while, so i wont run the whole thing again now, but it goes through now atleast.

Haha. I usually leave it run overnight so that I can do things elsewhere.

Yeah. Please proceed. Once you updated it please add the co-authored-by: signed off and raise a pull request for this one like https://github.com/hollowaykeanho/Upscaler/commit/fcf3d6133617feca5618dc997449f13b0b2fe518. Reason:

Appearently, GitHub only recognizes pair-progrmaming via its pull request so it wasn't a choice. =.=" I can't leave you empty handed in return. Not my style.

Don't worry, I will deal with the signing signatures after the pull request.

[Joly0]

Line number?

https://github.com/hollowaykeanho/Upscaler/blob/fcf3d6133617feca5618dc997449f13b0b2fe518/init/unix.sh#L502-L513C1

Yeah. Please proceed. Once you updated it please add the co-authored-by: signed off and raise a pull request for this one like fcf3d61.

Ok, so i have already commited and pushed prior to your comment. What do i have to add where to fulfill the github needs now?

[hollowaykeanho]

Hmm, just in case... please add me into your repo. I'll push some stuff before accepting the pull request.

UPDATE: I have a commit ready to push into your repo. Standing by waiting for your user addition. Please remember to set me as editor role.

[hollowaykeanho]

https://github.com/hollowaykeanho/Upscaler/blob/fcf3d6133617feca5618dc997449f13b0b2fe518/init/unix.sh#L502-L513C1

Oh that's not duplicate. FYI, FFMEG's input has a positional meaning. What it implies there is to take both the video and the upscale frame images as simultenous inputs where the video is the primary reference. Then apply the "resizing frame onto the primary reference + map the upscaled frame image in it" filter in accordingly.

That's how I saved 1 decoding cycle instead of using dissection method like everyone did (extract frames, then extract audio, then reassemble).

No worries.

[hollowaykeanho]

Standing by waiting for your user addition. Please remember to set me as editor role.

Granting access --> Repo's settings > Collaborator > add "hollowaykeanho" > set to edit permission. This should be fine. I'll handle the rest for you. =)

UPDATE:

Alternatively, you can git am PATCHFILE attached here.

0001-README.md-updated-with-Windows-instructions.patch.txt 0002-README.md-updated-instructions-and-touch-up-document.patch.txt

Then raise a pull request.

Please prioritize your vacation. I can wait.

[hollowaykeanho]

Thank you. Here you go! =) I'll rebase and re-write all the commits to match the latest.

UPDATE: I rebased the main branch already. You can delete the fork now and fork a new one in the future.

[hollowaykeanho]

@Joly0, we managed to run the test with the PowerShell script but appearently the init/windows.ps1 is not able to call the binary file properly shown below:

where the binary is working fine on the system (see below):

Are the arguments passing done correctly when calling the binary compared to bash (e.g. -f instead of --format etc)?

I just repaired the start.cmd alongside with its permission thing. It should be ok.

I'll try my best to repair the script at the moment. Focus on your vacation. You need it.

[Joly0]

Hm, it worked for me. Why do you think it isnt working correctly? For the powershell script i decided to hide the program window entirely because it is short-lived and doesnt give a lot of useful information. To see if it actually works (which it should) you could remove "-WindowStyle Hidden" here https://github.com/hollowaykeanho/Upscaler/blob/291d08d9cfef6f7226c7ac63d3e09eb543cda37f/init/windows.ps1#L186

[hollowaykeanho]

I just realized powershell don't provide feedback using Start-Process cmdlet from (https://lazyadmin.nl/powershell/start-process/). You're correct to hide the windows. For what I know, it will popup a new terminal.

I just altered the invoke instruction into

+       & "$program" @(
+               "-i", $input_path,
+               "-o", $output_path,
+               "-s", $scale,
+               "-m", $repo"\models",
+               "-n", $model,
+               "-f", $format
+       )

Currently testing with my friend. The binary feedback is very important. It shows GPU types and progression. That's what triggered us to think it's not working properly.

NOTE: with a temporary git commit. Will remove later and add a proper one.

[Joly0]

Would be smart to add

if ($LastExitCode) { Exit } after the command.

Maybe Write-Host with the error message before exiting, but i would have to check, how to properly output the error message of an external programm invoked with &

[hollowaykeanho]

Would be smart to add

if ($LastExitCode) { Exit } after the command.

You already got it covered. =)

Maybe Write-Host with the error message before exiting, but i would have to check, how to properly output the error message of an external programm invoked with &

PowerShell can't stream the binary's output real time? That's interesting. I was stunned and speechless after reading through that blog post.

Still pending my friend's test result before I rebase another progress.

My next attempt would be Invoke-Command if fails.. from here https://stackoverflow.com/questions/50995058/powershell-capture-output-from-invoke-command-running-exe-on-remote-machine

Will keep you posted.

[Joly0]

PowerShell can't stream the binary's output real time? That's interesting. I was stunned and speechless after reading through that blog post.

I mean, powershell can, but start-process cant. Powershell can sometimes be terrible with executing third party applications via cli and catch their output. There are multiple different ways like . and & or invoke-expression, invoke-command, start-process. All of them have their advantages and drawbacks. I sticked with start-process as it was simple and i didnt thought the output would be necessary, but you are totally right, that it is important.

But ye, invoke-command might be the best to handle the stream imo Tbh, batch is even worse in my opinion, powershell is better in every way. I couldnt have done this so easily with batch, so we must find a way to handle this on powershell

[hollowaykeanho]

But ye, invoke-command might be the best to handle the stream imo Tbh, batch is even worse in my opinion, powershell is better in every way. I couldnt have done this so easily with batch, so we must find a way to handle this on powershell

No worries. We are definitely staying in PowerShell. All we need to do is to replace that Start-Process with something sensible. I'm reverting the patch but I need your help on this one since you have a Windows machine over there (can be a lot faster). The patch I did was as below:

commit 2e0d5aecc5960f97f9e94db4f2fe0cfd4a94e23a
Author: (Holloway) Chew, Kean Ho <hollowaykeanho@gmail.com>
Date:   Sun May 21 17:51:06 2023 +0800

    S

diff --git a/init/windows.ps1 b/init/windows.ps1
index e050186..2120241 100644
--- a/init/windows.ps1
+++ b/init/windows.ps1
@@ -182,8 +182,14 @@ function Get-IO {
 }

 function Invoke-Upscale($input_path, $output_path) {
-   $params = "-i `"$input_path`" -o `"$output_path`" -s $scale -m `"$repo\models`" -n $model -f $format"
-   Start-Process -FilePath $program -ArgumentList $params -Wait -WindowStyle Hidden
+   & "$program" @(
+       "-i", $input_path,
+       "-o", $output_path,
+       "-s", $scale,
+       "-m", $repo"\models",
+       "-n", $model,
+       "-f", $format
+   )
 }

 function Invoke-Program {
@@ -342,4 +348,4 @@ if (-not (Get-IO) ) {

 if (-not (Invoke-Program) ) {
    Exit
-}
\ No newline at end of file
+}

Remember, it has to be piped in since the binary is reporting progress, not the result capturing like most SO recommended =.=".

No test results yet. Both of us collapsed early yesterday night =.=" too tired.

UPDATE: you may need to:

$ git reset 264ffb03a8da02566866b6cb8f8fb44789e55e93
$ git checkout .
$ git clean -fd
$ git pull

[corygalyna]

Hey, dropping to give you 2 some cheers! =)

[Joly0]

So i have tried multiple things now, using &, invoke-command, etc. But none of them really suited the case really. Either the output is not shown at all, as powershell can only redirect output of an executable to a variable without displaying it, or displaying the output, but cant redirecting it to a variable for error handling. Also & in my opinion results in a cluttered output, as it outputs to the same window, start-process opens a new powershell window.

In the end, i think this should work:

function Invoke-Upscale($input_path, $output_path) {
    $params = "-i `"$input_path`" -o `"$output_path`" -s $scale -m `"$repo\models`" -n $mode    l -f $format"
    $process = (Start-Process -FilePath $program -ArgumentList $params -Wait -PassThru)
    if ($process.ExitCode) {
        Write-Host "Error occured while upscaling:" $input_path
        Exit
    }
}

(for the paths the backticks before " are needed, had to take them out for github code highlighting) That way a new powershell window is opened to execute the program, the output is shown properly and we can exit the whole process if the upscaler errors-out. In my opinion thats the most elegant solution for this.

[hollowaykeanho]

I repaired your code snippet. Try use triple back ticks next time. Super useful. =)

I committed your changes already here in this patch: dafc12f655ad393f24d8ce66125db29dfce285a8

Will have my friend to run a test again and then benchmark it. I'll keep you posted.

[hollowaykeanho]

@Joly0 , would you mind help me run the tests/benchmark.cmd overnight and update me with 2 data please?

1. Statistics
2. Graphics card, VRAM, CPU type and RAM size

1 local set will do for this weekend's release. Hands are way too tight here. =x

[Joly0]

Sure, can do that

[Joly0]

Btw, i am curious why you change this https://github.com/hollowaykeanho/Upscaler/blob/331a23f773b4e9b8bcb963dd97bf3854179b4f69/init/windows.ps1#L206-L210 ?

[hollowaykeanho]

Ooh... make it common output across terminals... easier to communicate with end-users.

[Joly0]

Hm, understandable, but imo unnessecary to tell the user that video mode is not set. I mean, the user should know that and it makes the output more cluttered. Especially the formatting is not great imo. If output for both scenarios is desired, why not simply "Write-Host "Video mode set"" and "Write-Host "Video mode is not set"". The current way makes it unintuitive to read for me

[hollowaykeanho]

Hmm.. what about this one (generated from UNIX side replacing it to Yes/No)? If it works I do want to replace max scale with the real infinity unicode if allows.

I'm open for status reporting re-design. Not that hard compared to what we been through.

At the moment I'm dumping all the vitals for user to do sharp decision quick.

the user should know that and it makes the output more cluttered.

HAHAHA. End users. They're quite creative.

[Joly0]

Hmm.. what about this one (generated from UNIX side replacing it to Yes/No)? If it works I do want to replace max scale with the real infinity unicode if allows.

Hm, that looks good to me. Easy to read, understandable, not cluttered

I'm open for status reporting re-design. Not that hard compared to what we been through.

Haha, ye, maybe we first get this feature done right and working, afterwards we can discuss such things 😆

HAHAHA. End users. They're quite creative.

You are sooo right

[hollowaykeanho]

it makes the output more cluttered.

Hm, that looks good to me. Easy to read, understandable, not cluttered

Is the window side printing out the same or similar output at the moment? 0.o I'm assuming those

Write-Host ""

acutally spawns an empty line (\n), right? Otherwise, I need your help to patch it. XD

In that case, I'll push out the Yes/No and N/A printout first on UNIX side to your branch first for you to raise pull request.

Site-note: I managed to pump your pair badge to bronze.

Haha, ye, maybe we first get this feature done right and working, afterwards we can discuss such things

Should be fine now... we're hitting the project limit too. Too bad this project can't execute safe concurrency without actively monitoring the OS resources. It's a different beast.

[hollowaykeanho]

Ok.. I just pushed to your forked repo: f5d0afd8faf760381677e6036135d7ec1dbe0d64

Final terminal output would be:

Image

Video (start fresh)

Video (continue)

[Joly0]

Hm, looks like i have to take a look first at the last ffmpeg command. Somethings not right there

[hollowaykeanho]

pathing problem... missing frames after the _workspace directory. Full path should be:

tests\videos\sample-1-640x360-upscaled_workspace\frames\0%d.webp

[Joly0]

Ah, you are right. I will test it again later. My gf has something planned for our last vacation day, so i will report back later today with my results.

[Joly0]

Btw, i found some other bugs aswell and fixed them locally. I am currently running the benchmark to see if everything runs through. If it does, i´ll push my changes to my fork for you and post my results and specs here

[Joly0]

Ok, i dont know, but this never finished in 61 seconds So i guess somethings wrong with the time calculation

[Joly0]

Ok, i got it right i guess. Took some code from here https://gist.github.com/jcefoli/57881d79aa4c7548324e

[hollowaykeanho]

Ok, i dont know, but this never finished in 61 seconds

Yeap. Definitely a bug. It means on unix side we will have problems too. =.="

I don't think it's the timestamp collection part that is giving an error. It's very likely the calculator. Will give a run tonight and observe the pattern on UNIX side. Likely a ptach will push to you.

[Joly0]

Ok, i have pushed my fixes, but my calculation formula was wrong aswell, so i left it as is for now.

I am going to sleep now, if you find something i can test that tomorrow, if not, i will probably have time to look into it tomorrow afternoon when i am back at home.

[hollowaykeanho]

Ok, i have pushed my fixes, but my calculation formula was wrong aswell, so i left it as is for now.

Got it.. I'll handle it here and pint you back.

I am going to sleep now, if you find something i can test that tomorrow, if not, i will probably have time to look into it tomorrow afternoon when i am back at home.

Thank you. I'll keep you posted. Have a great rest!

[hollowaykeanho]

I applied https://stackoverflow.com/questions/673523/how-do-i-measure-execution-time-of-a-command-on-the-windows-command-line to fix the benchmark.cmd timing calculation problem (Hopefully it works).

Note that I fixes your commit (some leftover whitespace and commit messages) so you have to rebase your local repo to match your latest upstream.

I will give the UNIX part run again tonight (my time).

UPDATE: while at it, please raise a pull request but don't merge first. We will merge in once everything is hardened and testing working.

[Joly0]

Ok, it did run through.

This was running in a vm on my local server, i might be able to test it on my laptop-egpu combo later aswell. But the specs of the vm is GPU: Nvidia Quadro T600 (GTX 1650 equivalent) VRAM: 4GB CPU: Ryzen 9 7950x (but only with 8 cores + SMT; normally this would be 16 cores with SMT). Equivalent would be a Ryzen 7 7800X RAM: 22GB

[hollowaykeanho]

Haha. It's okay to release now with the statistics. Please be my guest, raise a pull request. I'll be happy to merge into the main repo anytime.

Vielen Dank für Ihre Mühe (not chatGPT; took me a while). =)

Wow. 3 times faster than my laptop.

i might be able to test it on my laptop-egpu combo later aswell.

Yeap. Sure. The more stat the better. I suggest we open a new ticket for this transaction.

[Joly0]

Ok, so i open a simple pr from my fork to your origin repo? What about the signing? Wasnt this an issue?