Closed gerito1 closed 9 years ago
The code it's functionally the same as the previous commit, but now I don't create a new thread. I thought, for some reason, it was necesary to avoid some blocking situation, I was wrong.
@radujipa this PR came before yours. Would you mind merging it to create a single one?
cc @gerito1
@alex5imon There are a few changes to how @gerito1 attempted a fix, specifically in windows/disk.py which might break it. Also windows/burn.py is slightly different. I wouldn't mind merging otherwise.
@gerito1 what are your thoughts here?
@alex5imon The Pull request of @radujipa obviously supersedes mine so nothing more to say here I guess.
The only fault I see over his Pull Request is that he split the burning process in extracting and burning, that is an increment in over 2GB in the requirements, so perhaps he should not do that.
But is an admirable work.
Merge with mine is irrelevant now.
@gerito1 The reason for the split in the end was that apparently Windows does not like pipes very much. I agree with you in that it would've been nice to have, and is the reason why I tried so many 'magic' tricks to get it to work. And it was working on Windows 7, but versions 8 and 8.1 did not like it one bit. It's a necessary evil.
I was excited when I first saw your threading solution though, and really thought it could work. :+1: It is just weird what was going on there to be honest.. It's been bugging me for months!
I though the same. The threading works, I rolled back that because It seemed redundant to me.
Try my last version, and see if that works. If does then nothing to say, just use the pipe but if it does not then in a couple of hours I will roll back again, merge mine with yours and add a Qt4 native thread solution.
Right now I'm busy sorry. @radujipa
@radujipa Did you tested that? The threaded code is still relevant? (see my previous comment).
@gerito1 I cloned your repo and did some tests in VirtualBox. My setup:
Mac OS 10.10.2
VirtualBox 4.3.20
Drive: SanDisk Cruzer Edge USB 16GB
Running from source
Windows 7: 1/1 successful
Windows 8: 2/2 successful
Windows 8.1: 4/4 failed
Apparently it is the same problem we were having since a few months ago. Because I cannot point any fingers either at Windows for doing different things in the shell with pipes, or at dd.exe
for not working well with stdin, I believe the most reliable thing to do is get rid of the pipe. Without it, we don't need any magic test writes to enable the disk to whatever mount point or partition.
The added space requirement I don't think should be of any major concern anyway since the temp directory is in C:\
and if you have less than 3GB free space there, then you're already facing OS instabilities, from my experience.
@radujipa That's weird works on my 8.1. You had any success with the previous commit, the one with the threads. I finally got to my desk.
@radujipa Ok I merged most of your code into mine. This has the thread version. See what you can do with it, is up to you now.
@gerito1 I made one last attempt at trying the pipe version with your latest master
It worked once, but then failed consistently. I'm sorry, but I won't be working on pipe versions as it doesn't seem reliable enough and, again, I do not know what is causing the problem - either the pipe or dd with stdin.
@radujipa Well ok then. In my laptop works consistently, I don't know what to say. I don't have other hardware to test. If I can't replicate I can't make a diagnosis. Feel free to do as you wish. Sorry for the trouble then.
@alex5imon Nothing more to say here. It seems that there are problems that go beyond me. I'm closing this now.
@gerito1 thanks for your input!
This patch should fix the kano-burner for windows. (7, 8 and 8.1)
This should be fully functional now.