Windows fixes and Timeshift buffer deletion #116
Conversation
|
@Rechi let me know if this fixes the windows build problem. |
|
Ok I've pushed again. Still no clue on why the build is failing on AutoTimer.cpp, it's like it can't see it's own header file. @ksooo any ideas? |
|
Show me the error message, please. |
There was a problem hiding this comment.
You should really be very careful with all these locks you have introduced with this PR! There is high deadlock potential and we are nearly in Leia RC phase. I have no good feelings with such a dangerous change at that stage. Did you used the golden hammer instead of fixing the actual problem?
|
@ksooo look into the jenkins pr status check. |
The type seems not to be found. Missing include? namespace problem? |
|
Don't I feel like an idiot. Spent hours blindly looking at the code and it's obvious now. Thanks @ksooo As the lock code is controversial I will remove it from this PR so we can get the build back working. I'll put it in a separate PR for discussion. What I need with it is some guidance on how to resolve the issue. |
Sounds like a good plan. Thanks. |
I know that feeling... :-) |
phunkyfish
changed the title
|
Ok, should be good now. I'm curious as to why the namespace issues were a problem in Windows but not on any other platform. They were in fact incorrect just funny that no other platform seemed to care. |
|
@ksooo I don't appear to be able to resolve you previous review. Maybe as I had already pushed the branch. |
|
@ksooo for the change in Epg.cpp any reason not to use char instead of byte? |
ksooo
dismissed
their stale review
locks are removed from this PR, so my -1 is no longer valid
No reason. |
|
Ok, I change the DeleteFile call to a condition instead. Also, I removed ifdef WIN32 from Enigma2.cpp as it's no longer required. Hopefully, we are good now. |
|
Ok, so I’ll revert again.
… On 16 Oct 2018, at 09:51, Rechi ***@***.***> wrote:
@Rechi commented on this pull request.
In src/enigma2/TimeshiftBuffer.cpp:
> @@ -39,6 +39,10 @@ TimeshiftBuffer::~TimeshiftBuffer(void)
}
if (m_filebufferReadHandle)
XBMC->CloseFile(m_filebufferReadHandle);
+
+ if (!XBMC->DeleteFile(m_bufferPath.c_str()))
As this currently compiles, you don't have to care about it.
It's needed in pvr.vbox as windows headers are pulled in somewhere.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Ok, I'm remove the WIN32 undef. Should be finally, finally ready to go ;) |
|
No worries, sure we’ll suck it and see.
… On 16 Oct 2018, at 14:51, Sam Stenvall ***@***.***> wrote:
@Jalle19 commented on this pull request.
In src/enigma2/TimeshiftBuffer.cpp:
> @@ -39,6 +39,10 @@ TimeshiftBuffer::~TimeshiftBuffer(void)
}
if (m_filebufferReadHandle)
XBMC->CloseFile(m_filebufferReadHandle);
+
+ if (!XBMC->DeleteFile(m_bufferPath.c_str()))
I honestly can't remember
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or mute the thread.
|
v3.12.6
First appears to be a race condition on addon startup, I have to start the addon about 50 times to get it to happen. Welcome any feedback how I added the lock objects. I referenced several other addons to see how it was done there so I think have everything covered. On the third the Epg.cpp fixed worked bu now's it's stuck on AutoTimer.cpp