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

Already on GitHub? Sign in to your account

Update outdated buffer size value in File.cpp #1260

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Member

Karlson2k commented Aug 7, 2012

Again get rid of xbox.

Member

elupus commented Aug 7, 2012

Should be fine, but I doubt it will make a noticeable difference.
On Aug 7, 2012 5:24 PM, "Karlson2k" notifications@github.com wrote:

256k seems to be optimal for windows platform (see various ATTO results on

the web).

You can merge this Pull Request by running:

git pull https://github.com/Karlson2k/xbmc CFile_buffer_size_update

Or view, comment on, or merge it at:

#1260
Commit Summary

  • Update outdated buffer size value in File.cpp

File Changes

  • M xbmc/filesystem/File.cpp (5)

Patch Links

Member

Karlson2k commented Aug 7, 2012

@elupus Agree about noticeable difference. Main goal of this commit was removing xbox relation.

Member

jmarshallnz commented Aug 7, 2012

We don't just run on Windows though, so unless it has a beneficial effect on all platforms, this is a no-go.

Member

Karlson2k commented Aug 7, 2012

@jmarshallnz There is placeholder for other platforms, but it's common on all platforms - bigger buffer results in faster copy.
So I'm sure that it will improve performance on all platforms.

But I can #ifdef for now-Windows platform with old (xbox) value. Should I do so?

Member

jmarshallnz commented Aug 7, 2012

So you've actually done tests on a number of different
filesystems/protocols (smb/afp/local etc.) with a number of differently
sized files over a number of different network types (wired, wifi, local)?

On Wed, Aug 8, 2012 at 9:44 AM, Karlson2k notifications@github.com wrote:

@jmarshallnz https://github.com/jmarshallnz There is placeholder for
other platforms, but it's common on all platforms - bigger buffer results
in faster copy.
So I'm sure that it will improve performance on all platforms.

But I can #ifdef for now-Windows platform with old (xbox) value. Should I
do so?


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/1260#issuecomment-7567902.

Member

elupus commented Aug 8, 2012

(drunk off my ass) if this would lower performance, I would be a smurf.

Drop the "optimal" comment (comment altogether) then at least I'm fine with
it.
On Aug 7, 2012 10:48 PM, "jmarshallnz" notifications@github.com wrote:

So you've actually done tests on a number of different
filesystems/protocols (smb/afp/local etc.) with a number of differently
sized files over a number of different network types (wired, wifi, local)?

On Wed, Aug 8, 2012 at 9:44 AM, Karlson2k notifications@github.com
wrote:

@jmarshallnz https://github.com/jmarshallnz There is placeholder for
other platforms, but it's common on all platforms - bigger buffer
results
in faster copy.
So I'm sure that it will improve performance on all platforms.

But I can #ifdef for now-Windows platform with old (xbox) value. Should
I
do so?


Reply to this email directly or view it on GitHub<
https://github.com/xbmc/xbmc/pull/1260#issuecomment-7567902>.


Reply to this email directly or view it on GitHubhttps://github.com/xbmc/xbmc/pull/1260#issuecomment-7567996.

Member

Karlson2k commented Aug 8, 2012

@elupus Done. :)

Member

Karlson2k commented Aug 10, 2012

@elupus @jmarshallnz Are we still in merge window?

Member

elupus commented Aug 10, 2012

Somewhat. I can't merge though.

Member

jmarshallnz commented Oct 13, 2012

I've removed the comment in master. Unless there's some good profiling showing that 256k is be significantly than 128k (it can't really be worse), no need for this.

@Karlson2k Karlson2k deleted the Karlson2k:CFile_buffer_size_update branch Oct 15, 2013

@tru tru added a commit to plexinc/plex-home-theater-public that referenced this pull request Jul 11, 2014

@tru tru Fix advancing video playQueues.
There where a few fishy things here.

First the debug log was totally wrong and lied to me which
made me a bit confused. That was because I was using the wrong
variable in to show what playlist was in use.

Then I added the playQueueItemID to the timelines going to the
server as well. This makes the code on the server lookup more
reliable.

Lastly the media item that we got from the MDE had the wrong itemId
because it was using ratingKey instead of playQueueItemID.

Fixes #1260
984c59b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment