2014-04-08, 19:32
How would I go about adding this patch to the android version
(2014-04-08, 19:32)ngoal Wrote: How would I go about adding this patch to the android version
(2014-04-08, 17:55)althekiller Wrote: There's only one point that there are three CReadStates in play there, lines 1172-1210. Give this patch a try: http://nopaste.linux-dev.org/?147629. It eliminates the third CReadState instance, but I didn't really check for repercussions.Thanks, I'll do some testing of your patch tomorrow versus disabling multisession entirely as in my hack.
(2014-04-08, 19:32)ngoal Wrote: How would I go about adding this patch to the android versionSorry, can't help you there. I have no android devices and know nothing about how to compile an android version of XBMC. I linked to the source code of the patch on github above so anyone who can compile the android version could easily compile that version.
(2014-04-08, 17:55)althekiller Wrote: There's only one point that there are three CReadStates in play there, lines 1172-1210. Give this patch a try: http://nopaste.linux-dev.org/?147629. It eliminates the third CReadState instance, but I didn't really check for repercussions.Just tried this and it does reduce maximum simultaneous connection to two which certainly improves matters, (but isn't as robust as turning multisession off entirely with limited connections available) but in the process its reverting a change deliberately made in ulion's original commit. I'm trying to understand what the benefit of having up to three simultaneous connections is, why it was done, and what the drawback of limiting it to two connections might be ?
long response = m_state->Connect(m_bufferSize);
if(response < 0 && (m_state->m_fileSize == 0 || m_state->m_fileSize != m_state->m_filePos))
{
m_seekable = false;
if(m_multisession && m_oldState)
{
delete m_state;
m_state = m_oldState;
m_oldState = oldstate;
}
return -1;
}
(2014-04-09, 14:43)Kib Wrote: Could you not instead of setting m_seekable to false, check if m_multisession is true, and set to false instead?That by itself wouldn't be enough - the connection for the seek failed to return any data so as well as marking m_multisession false (so we don't try to use extra sessions for the next seek) we need to find the handle of a previous curl session and reuse it to retry the request, or close a previous idle connection handle then open a new one to keep the connection count down. (It's still not clear to me exactly how the code works - I'm doing a lot of single stepping while monitoring the network connections at the server end to get to grips with it )
If not true keep old way of working?
Quote:I'm not a coder so ignore if this is uselessI'm not a coder either, I just pretend to be occasionally...
(2014-04-09, 15:13)wsnipex Wrote: I guess we need to keep track of how many concurrent connections are allowed with a counter and a max of sessions.There is a maximum of three sessions now due to the following code rotating stuff through m_state, m_oldstate, and oldstate:
We shouldn't even set multisession to false in case a 3rd connection fails, only if a 2nd fails.
(2014-04-09, 15:40)althekiller Wrote: Well we need the patch I posted regardless as there's the possibility to leak oldstate on that silly early return. I agree that disabling multi session if a second connection fails is probably the right way to go.Ah good, so you think limiting it to a maximum of two as per your patch is preferable, and that there's no good reason to use as many as three ?
(2014-04-09, 15:40)althekiller Wrote: Well we need the patch I posted regardless as there's the possibility to leak oldstate on that silly early return. I agree that disabling multi session if a second connection fails is probably the right way to go.Might have something here, only about 3 lines changed beyond what you changed, and it seems to work, but I need someone that understands the code properly to sanity check it: