Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Networkcache #831

Closed
wants to merge 2 commits into from
Closed

Networkcache #831

wants to merge 2 commits into from

Conversation

bobo1on1
Copy link
Member

  1. Added an option to do cached reads from the LAN, with the buffer size based on free ram.
  2. Increase max readrate based on audio and video bitrates.

Combined these commits help with high bitrate video playback over WiFi and HomePlug networks.

bobo1on1 added 2 commits March 30, 2012 02:31
…when enabled this will do cached reads from the lan, using 50% of free ram as buffer, with 25 mb as lower and 512 as upper limits
@DDDamian
Copy link
Contributor

+1 to this

@jmarshallnz
Copy link
Contributor

-1 on any new settings. I'm sure we can enable better cache management by default without user interaction being required.

@DDDamian
Copy link
Contributor

Maybe just drop the GUI setting and leave the advancedsettings? As long as they're reasonable defaults for minimal memory platforms then no user interaction is required, but for those with more memory available they can make use of it. The default max of 512mb may be a bit high for some platforms, but for users with other people (kids) sharing their networks it makes sense to grab what you can while you can.

@arnova
Copy link
Member

arnova commented Mar 30, 2012

Basically what jmarshallnz says: Might as well do this for internet-streams too and make it the default. I don't see why we need to differentiate between LAN and internet stuff. Might also pay to ask Elupus's opinion on this...

@MartijnKaijser
Copy link
Member

I would vote to minimize number of option as well. Already to many. Like already suggested make it default and if they want users can always disable through as.xml

@bobo1on1
Copy link
Member Author

The things we have to consider are:

  • When playing over a wired network there's no need to buffer, so it will just waste cpu and memory.
  • When buffering too quickly, the network will slow down and will even severely increase cpu usage if one has a gigabit network.
  • When buffering too slowly, the cache won't fill quickly enough to compensate for high bitrate scenes.

What I'm trying to solve is the situation where one has a network connection that on average is fast enough for playing a certain file, but is not fast enough for high bitrate scenes, in which case you want to read and buffer as much as possible.
I'm not too concerned with ram usage, modern systems have plenty of it and we can detect the amount of free ram, cpu usage on my system increases from 32% to 40% when the cache is enabled, but I think some things there can be optimized a little.
So that leaves choosing a good read speed, the algorithm I used here based on peak bitrate still chooses quite a high read speed, and will quickly max out the wifi or internet connection.

@DDDamian
Copy link
Contributor

Agree with Arnova - makes sense for all net streams, and there's a good argument for all caching. If it's not on the machine the purpose is to get it to the machine as quickly as possible without a usability penalty. You could get as fancy as using the CPU usage as one of the factors in setting the read rate.if you want it to scale better to it's environment while still being "transparent".

@elupus
Copy link
Contributor

elupus commented Apr 10, 2012

-1 on new settings. That also is a -1 on advanced settings. Imho we should just bump the bitrate limit on the CFileCache to something more than 1 mbit above video.

Just setting it at 5mbit above video aught to be fine.. Means we have 5mbit per second of video playback. Thus after 3 seconds of intro it should cope with a 15mbit increase in video.. that shouldn't happen often.

@bobo1on1
Copy link
Member Author

twice the average bitrate also seems to work nicely.

@Jellyfrog
Copy link

+50 on this, I'm unable to use xbmc because of the lack of buffering for "LAN", since I connect to my sources with VPN, and the tunnel sometimes drops in speed, the video stops and it says "buffering" for 2 sec, then plays for 10sec then "buffering" again...

Maybe add the option to buffer or not per source?

@DDDamian
Copy link
Contributor

DDDamian commented May 3, 2012

Bump for comments/suggestions. At idle on my machine XBMC is using 82mb out of (being 32bit) the 4GB available to it. Playing a 1080p with TrueHD @ ~8mb/sec it's using 145mb of ram.

I think this has value if implementation can be agreed on. I see no reason to limit it to any connection type either.

@bobo1on1
Copy link
Member Author

bobo1on1 commented May 3, 2012

I've discovered that smb sometimes breaks when this is enabled, and it requires a restart of XBMC to fix it, so no go for now.

@DDDamian
Copy link
Contributor

DDDamian commented May 3, 2012

np - just think it's a step in the right direction and didn't want to see it dropped :)

@arnova
Copy link
Member

arnova commented May 26, 2012

Perhaps we could already implement it the way it is for our Curl filesystem? In that way we could at least already test it?

@arnova
Copy link
Member

arnova commented May 26, 2012

Oh and rebase please ;-)

@classicspam
Copy link
Contributor

Was pointed to this pull request after I put in my pull requests. Some thoughts:

-- SMB issues...this might be caused by the rate at which you are trying to read. At 3X max bitrate of the movie the read rate can get quite high and I have noticed issues like audio getting slightly out of sync and choppiness when setting artificially high read rates which was resolved by lowering the read rate. Perhaps limiting to max bitrate would help SMB issues for example:

void CDVDPlayer::UpdateReadRate()
{
unsigned int bytespersecond = (GetVideoBitrate() + GetAudioBitrate()) / 8;

if (bytespersecond > m_readrate * 1.25)
{
m_readrate = bytespersecond;
m_pInputStream->SetReadRate(m_readrate);
}
}

would up the read rate until it is 75%+ of the max read rate. This may solve the smb issues.

-- Probably do not need the advanced settings "maxcachesize" and just use the cachemembuffersize as the max size or even better yet remove them both and create a different advanced settings variable called "freememorycachepercentage" with default 50% (0 percent would force file cache)

-- In FileCache.cpp the equation should probably something like:

unsigned int cacheram = calculated buffer size
unsigned int front = cacheram * 0.75;
unsigned int back = cacheram - front;

m_pCache = new CCircularCache(front
, std::max( back, 1024 * 1024));

-- make it default i.e. use default constructor

-- if the setting to force buffer for smb files cannot be put into into the GUI then an advanced settings variable should be used...this is the one this that I think the user should have an option on as people with lan's will probably not need to use this for smb files.

This will defiantly be better than reading at average bitrate when using caching.

@classicspam
Copy link
Contributor

Ok I merged some of the changes from this pull request and added bitrate limiting functionality in my "Branch_SMBUseBuffer" branch and rebased to a current master.

it currently uses advanced settings to set items up in the network section:

added:

smbforcebuffer - boolean forces xbmc to use buffer for smb files (default false)
freememorycachepercent - percentage of free ram to use for cache (default 50, max 80, 0 causes xbmc to use file buffer, max cache size hard limited to 1 GB)

removed:

cachemembuffersize - no longer needed as cache is based on percent free ram and not an amount

I cannot get it to crash on smb files (or ftp, dav, http) so if anybody would like to help test to try and crash it, it would be appreciated...

@michaelbaudino
Copy link

This feature may be useful on non-SMB files too : I'm reading videos over internet using a SSHFS mount (as described on this XBMC forum thread), which seems to be viewed as a local FS by XBMC.
And as my internet connection is not always as performant as I would like to (e.g. not able to stream not-so-compressed 720p movies), a buffer to download data prior to reading it (maybe only a few minutes prior), would be highly useful !

And, on a more philosophical level, with the rise of personnal cloud storage, people may tend to store and stream more and more videos over-the-internet in the future.

@classicspam
Copy link
Contributor

Well I updated my branch with the following:

  1. Switch advanced settings variable from "smbforcebuffer" to "alwaysforcebuffer"
  2. When this setting is enabled it will force the buffer on everything except Pictures or IsOnDVD or IsBluRay (i.e. dvd rom drive unless I am mistaken on the IsBluRay functionality). This should force the buffer on OS mounted network drives (also on local hard drives, flash drives, etc as a result).

Again testing is welcome as it is not taking into account IsPlugin = false which IsOnLan takes into account (not sure if that check is relevant for using the buffer)

@michaelbaudino
Copy link

Thanks, I'm cloning it atm :-)
Is there a way to monitor cache level in real-time while playing a video to check that it's working ?
(I'll probably try that during the week-end)

@arnova
Copy link
Member

arnova commented Aug 10, 2012

You can see it's working by checking the OSD info (it's the cache: property) and you can also see it when popping up info during video playback, the seekbar will show a darker trailing part which is the progressive cache..

@michaelbaudino
Copy link

Nice, I'll let you know, then :-)

@michaelbaudino
Copy link

I've been using @classicspam 's branch for almost a month now, and it's working great ! Except for a few really big files (which I cannot watch seamlessly even with full cache), I had absolutely no problem.
One thing I noted, though, is that the cache's maximum capacity, may vary (sometimes it's 900MB, other times 150MB), but I guess it depends on the available memory which must be taken by another process (or by the very same process, btw).
@classicspam, do you plan to issue a pull-request to merge your branch in master ?

@classicspam
Copy link
Contributor

I can if it is alright with the devs as some of the code used in it is from this PR with the following additions:

  • move variables over to advanced settings ("alwaysforcebuffer" default false and "freememorycachepercent" default 50% max 80% with hard limit of 1GB. It also removes "cachemembuffersize" variable as it is no longer needed)
  • "alwaysforcebuffer" variable will cache everything run through dvdplayer (i.e. OS network shares, local media, etc) except Optical Media Drives
  • Memory buffer is straight percentage of free ram (i.e. if 50% free ram is used ~75% of the 50% will be forward looking buffer and ~25% of the 50% will be back buffer)
  • Rate limiting which fixes SMB issues as far as I can tell (1.25 time max bitrate up to 40 MB/s in which case it is throttled to Max Bitrate)
  • ios and linux fixes

@DDDamian
Copy link
Contributor

DDDamian commented Sep 6, 2012

Is there a forum thread where others have tested this? Just want to see if it's working well for several testers. Once you're sure please submit a PR (this one needs rebasing anyways), but make sure it's had testing on several platforms / protocols.

@arnova
Copy link
Member

arnova commented Sep 6, 2012

But what about the issues that bob1on1 was having with smb, which is the sole reason this never got merged... ?

@michaelbaudino
Copy link

I can't comment on that SMB bug, since I'm using SSHFS, but I did not encounter such a bug with SSHFS...

@classicspam
Copy link
Contributor

I have not had any issues with SMB with my branch after testing on windows/linux and atv2 for a month+...the cause of the issues may have been either trying to read too fast (i.e. 3X max bitrate) or the fact that this PR still uses 25% of cachemembuffer size variable as the back buffer in addition to percentage of free ram as the forward buffer...both items are taken care of in my branch (rate limiting and carving up the free ram percentage into back/forward buffers)...

@arnova
Copy link
Member

arnova commented Sep 7, 2012

Sounds good. Please submit a PR then for further inspection...

@doits
Copy link

doits commented Sep 23, 2012

I'm using patches based of the @classicspam branch with latest openelec frodo and buffering nearly works perfect with locally mounted nfs (over wlan) storage and alwaysforcebuffer.

Only one problem: A video does not begin to play at once after starting it. Looks like it wants to fill up some cache before - XBMC hangs at the video list for about 40-80 seconds after starting a video. When the video starts, it already has ~500mb in cache. Then everything works fine. Someone else got this, too?

@doits
Copy link

doits commented Sep 23, 2012

And one other thing: after starting and stopping to play four videos, XBMC is completely eating RAM. Not sure if this is supposed to, or shouldn't it free RAM after stopping a video?

             total         used         free       shared      buffers
Mem:          3960         3928           32            0           51
-/+ buffers:               3876           83
Swap:            0            0            0

@classicspam
Copy link
Contributor

@doits - I have noticed that on occasion it does try to fill up the buffer before playing the video over wlan (seems to happen for high bitrate videos)...pressing play while it is buffering starts the video as normal. Also I have not observed the memory leak can you try and use xbmc nfs network share instead of OS mounted to see if it still exhibits this leak? Can you verify if it also occurs on SMB?

@michaelbaudino
Copy link

I've seen that too. Also, sometimes, with pretty big videos (e.g. 7GB BRRIPs), the buffer fills up totally but a message pops-up at the end of buffering : something like (i don't remember the exact sentence) "the buffer is full, but it will not be sufficient to watch the video until the end".
So my guess is that while buffering, the player computes the amount of buffer necessary (depending on the buffering speed). I think the movie is launched when this required amount has been reached (or, if it is never reached, it displays the message I've been describing above).

Anyone can confirm such a behaviour ?

@doits
Copy link

doits commented Sep 23, 2012

@classicspam my problem happened without cache enabled, too, now ... cleared my xbmc setting totally and reconfigured it - works flawless new. sorry for the confusion.

@da-anda
Copy link
Member

da-anda commented Sep 30, 2012

what's the status of this? Just reading through the PRs and check what might be nice for Frodo. Thus this bump.

@doits
Copy link

doits commented Sep 30, 2012

using @classicspam branch without any problems last 5-8 movies. +1 for merge.

@michaelbaudino
Copy link

I'd +1 for merge too, if that helps...

@doits
Copy link

doits commented Oct 14, 2012

I'd be glad to hear something from the devs here. If there's something still not right with the patches to be included we'll work it out.

Without this, I couldn't watch 1080p videos (~10gb/h) without 1-2 disturbing "buffer-breaks" for few seconds per hour over my wlan. The "old" buffer did not catch extreme spikes in bitrate nor an unsteady wlan bitrate. With this, I have absolutely no problems. Videos even start at once (no "buffering on start" or so) and play until the end.

Another big benifit: small distance seeking is no hassle anymore! Before even skipping 20 seconds sometimes lasted 3-5 seconds, now seeking inside buffer range works at once.

I cannot see atm why this is not merged. If there's something needed to be worked out, please tell us.

Talking about @classicspam's patch with latest frodo builds, https://github.com/classicspam/xbmc/tree/Branch_SMBUseBuffer

Edit: I see, theres a PR #1388 already for this ... should be at least referenced like now.

@MartijnKaijser
Copy link
Member

since we are at feature freeze i doubt this would be pulled in and @classicspam should open a PR with his patches so it can be reviewed

@classicspam
Copy link
Contributor

PR #1388 (network cache redux)

@MartijnKaijser
Copy link
Member

See #3104

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet