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

VideoPlayer: init ffmpeg threads like LAVFilters does #9899

Merged
merged 1 commit into from
Jun 3, 2016

Conversation

FernetMenta
Copy link
Contributor

see title

@FernetMenta FernetMenta added Type: Improvement non-breaking change which improves existing functionality v17 Krypton Component: Video labels Jun 3, 2016
@FernetMenta FernetMenta added this to the Krypton 17.0-alpha2 milestone Jun 3, 2016
@afedchin
Copy link
Member

afedchin commented Jun 3, 2016

is this related to http://forum.kodi.tv/showthread.php?tid=277724?

@fritsch
Copy link
Member

fritsch commented Jun 3, 2016

@afedchin yes - that should allow up to 16 threads, while av_cpu_count returns the number of logical cores. E.g. 8 on a i7 quad core.

The *3 / 2 most likely was found by cpuload testing as it seems the decoder does not fully use the cpu cores. So running 50% more most likely has increased performance.

@afedchin
Copy link
Member

afedchin commented Jun 3, 2016

@fritsch thanks for clarification.

@FernetMenta
Copy link
Contributor Author

jenkins build this please

@fritsch fritsch merged commit 7132dcd into xbmc:master Jun 3, 2016
@MrMC
Copy link

MrMC commented Jun 3, 2016

can remove

#if defined(TARGET_POSIX) || defined(TARGET_WINDOWS)
#include "utils/CPUInfo.h"
#endif

@FernetMenta FernetMenta deleted the threads branch June 3, 2016 14:59
@FernetMenta
Copy link
Contributor Author

on my list

herrnst pushed a commit to herrnst/xbmc that referenced this pull request Jun 3, 2016
@MrMC
Copy link

MrMC commented Jun 7, 2016

fixed for android
MrMC/mrmc@602d6d9

@fritsch
Copy link
Member

fritsch commented Jun 7, 2016

@MrMC av_cpu_count <- does not work correctly or does it ignore sleeping CPUs which are not usable afterwards anyways?

@MrMC
Copy link

MrMC commented Jun 7, 2016

av_cpu_count only counts active cpus (for android). With the change, sleeping cpus will be woken and used. Tested under a quad.

@stefansaraev
Copy link
Contributor

that means, not quite working on linux if hotplug cpu scaling governor is used, too :)

@MrMC
Copy link

MrMC commented Jun 7, 2016

depends on if sched_getaffinity is present on linux and others.

@stefansaraev
Copy link
Contributor

uh. it is available of course, but it wont count "inactive" cpus (hotplug, as said..) ;)

however. I dont think it will be a big deal

@MrMC
Copy link

MrMC commented Jun 7, 2016

It can be a big deal depending on cpus active at time of call. Leads to unpredictable behavior for sw decoding that depends now on how many cpus are active at the time of the call. If you have all spun up, then can handle resource hungry formats, if only one then same media will max decode and fall behind.

@stefansaraev
Copy link
Contributor

stefansaraev commented Jun 7, 2016

ookay. then I guess hotplug will not be good choice anymore, I'll advice people switch back to ondemand.

btw, I just tested it. while /proc/cpuinfo says "4 cpus", sched_getaffinity sees 3 right now (most of the time it sees 1, if kodi is idle) :)

@FernetMenta
Copy link
Contributor Author

let's switch back to g_cpuInfo.getCPUCount() and multiply with 3 / 2

@stefansaraev
Copy link
Contributor

@FernetMenta most linux hw/distributions do not use hotplug, it's just wetek (and maybe some chineses) who do afaik. hotplug is not even available on mainline kernels. so up to you :)

guidosarducci pushed a commit to guidosarducci/xbmc that referenced this pull request Sep 3, 2016
herrnst pushed a commit to herrnst/xbmc that referenced this pull request Nov 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Type: Improvement non-breaking change which improves existing functionality v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants