| Message ID | 201705151240.24007.cehoyos@ag.or.at |
|---|---|
| State | Accepted |
| Commit | 17806f6083c888fbb1a35da6dd21c42a20e5a315 |
| Headers | show |
On Mon, 15 May 2017 12:40:23 +0200 Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > From 42766f345dbf398716c6fd9072f072f5fa91c940 Mon Sep 17 00:00:00 2001 > From: Steve Kondik <steve@cyngn.com> > Date: Tue, 16 Dec 2014 01:37:57 -0800 > Subject: [PATCH 2/2] avutil: Use _SC_NPROCESSORS_CONF > > * On most Android devices, CPUs can appear and disappear due to hotplug > or CPU cluster management. Use the total number of CPUs instead so > that multithreaded decoding is properly optimized. This explanation isn't really sufficient. Also not why you would want this, or why you would want this only on Android. > Change-Id: I1cbf000a1bda7b3abf0a84e971e752f176857385 what is this? > --- > libavutil/cpu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavutil/cpu.c b/libavutil/cpu.c > index 16e0c92..ab0965b 100644 > --- a/libavutil/cpu.c > +++ b/libavutil/cpu.c > @@ -282,6 +282,8 @@ int av_cpu_count(void) > > if (sysctl(mib, 2, &nb_cpus, &len, NULL, 0) == -1) > nb_cpus = 0; > +#elif defined(__ANDROID__) && HAVE_SYSCONF && defined(_SC_NPROCESSORS_CONF) > + nb_cpus = sysconf(_SC_NPROCESSORS_CONF); > #elif HAVE_SYSCONF && defined(_SC_NPROC_ONLN) > nb_cpus = sysconf(_SC_NPROC_ONLN); > #elif HAVE_SYSCONF && defined(_SC_NPROCESSORS_ONLN)
Hi, On Mon, May 15, 2017 at 6:53 AM, wm4 <nfxjfg@googlemail.com> wrote: > On Mon, 15 May 2017 12:40:23 +0200 > Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > > Change-Id: I1cbf000a1bda7b3abf0a84e971e752f176857385 > > what is this? Looks like a gerrit (patch review system) patch marker, see e.g.: http://www.webmproject.org/code/contribute/submitting-patches/#commit-msg-hook Ronald
2017-05-15 12:53 GMT+02:00 wm4 <nfxjfg@googlemail.com>: > On Mon, 15 May 2017 12:40:23 +0200 > Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > >> From 42766f345dbf398716c6fd9072f072f5fa91c940 Mon Sep 17 00:00:00 2001 >> From: Steve Kondik <steve@cyngn.com> >> Date: Tue, 16 Dec 2014 01:37:57 -0800 >> Subject: [PATCH 2/2] avutil: Use _SC_NPROCESSORS_CONF >> >> * On most Android devices, CPUs can appear and disappear due to hotplug >> or CPU cluster management. Use the total number of CPUs instead so >> that multithreaded decoding is properly optimized. > > This explanation isn't really sufficient. Please suggest a better one assuming you are objecting pushing as-is. > Also not why you would want > this, or why you would want this only on Android. The explanation sounded to me as if this would be Android-only. Carl Eugen
On Tue, 16 May 2017 12:40:03 +0200 Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2017-05-15 12:53 GMT+02:00 wm4 <nfxjfg@googlemail.com>: > > On Mon, 15 May 2017 12:40:23 +0200 > > Carl Eugen Hoyos <cehoyos@ag.or.at> wrote: > > > >> From 42766f345dbf398716c6fd9072f072f5fa91c940 Mon Sep 17 00:00:00 2001 > >> From: Steve Kondik <steve@cyngn.com> > >> Date: Tue, 16 Dec 2014 01:37:57 -0800 > >> Subject: [PATCH 2/2] avutil: Use _SC_NPROCESSORS_CONF > >> > >> * On most Android devices, CPUs can appear and disappear due to hotplug > >> or CPU cluster management. Use the total number of CPUs instead so > >> that multithreaded decoding is properly optimized. > > > > This explanation isn't really sufficient. > > Please suggest a better one assuming you are objecting pushing as-is. Yes, I'm objecting. I don't have to suggest a better one - you're the one who wants to push this. > > Also not why you would want > > this, or why you would want this only on Android. > > The explanation sounded to me as if this would be Android-only. The explanation is completely insufficient.
Hi, On 15/05/2017 12:40, Carl Eugen Hoyos wrote: > > 0002-avutil-Use-_SC_NPROCESSORS_CONF.patch > > > From 42766f345dbf398716c6fd9072f072f5fa91c940 Mon Sep 17 00:00:00 2001 > From: Steve Kondik <steve@cyngn.com> > Date: Tue, 16 Dec 2014 01:37:57 -0800 > Subject: [PATCH 2/2] avutil: Use _SC_NPROCESSORS_CONF > > * On most Android devices, CPUs can appear and disappear due to hotplug > or CPU cluster management. Use the total number of CPUs instead so > that multithreaded decoding is properly optimized. I'm not convinced the patch below fixes the issue described above. The idea is to optimize the number of threads given the number of usable CPUs. If a user of libav* wants to make sure they use the right number, they should make sure that the CPUs are woken up prior to calling this. There are ways to force the system's power management to make sure that the number of online CPUs is maximized, and, IMHO, that should be used instead of returning the total number of CPUs on the platform. > Change-Id: I1cbf000a1bda7b3abf0a84e971e752f176857385 > --- > libavutil/cpu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavutil/cpu.c b/libavutil/cpu.c > index 16e0c92..ab0965b 100644 > --- a/libavutil/cpu.c > +++ b/libavutil/cpu.c > @@ -282,6 +282,8 @@ int av_cpu_count(void) > > if (sysctl(mib, 2, &nb_cpus, &len, NULL, 0) == -1) > nb_cpus = 0; > +#elif defined(__ANDROID__) && HAVE_SYSCONF && defined(_SC_NPROCESSORS_CONF) > + nb_cpus = sysconf(_SC_NPROCESSORS_CONF); > #elif HAVE_SYSCONF && defined(_SC_NPROC_ONLN) > nb_cpus = sysconf(_SC_NPROC_ONLN); > #elif HAVE_SYSCONF && defined(_SC_NPROCESSORS_ONLN) > -- 1.7.10.4
diff --git a/libavutil/murmur3.c b/libavutil/murmur3.c index 4271e01..ef853f4 100644 --- a/libavutil/murmur3.c +++ b/libavutil/murmur3.c @@ -60,7 +60,7 @@ static uint64_t inline get_k1(const uint8_t *src) return k; } -static uint64_t inline get_k2(const uint8_t *src) +static inline uint64_t get_k2(const uint8_t *src) { uint64_t k = AV_RL64(src + 8); k *= c2; @@ -69,7 +69,7 @@ static uint64_t inline get_k2(const uint8_t *src) return k; } -static uint64_t inline update_h1(uint64_t k, uint64_t h1, uint64_t h2) +static inline uint64_t update_h1(uint64_t k, uint64_t h1, uint64_t h2) { k ^= h1; k = ROT(k, 27); @@ -79,7 +79,7 @@ static uint64_t inline update_h1(uint64_t k, uint64_t h1, uint64_t h2) return k; } -static uint64_t inline update_h2(uint64_t k, uint64_t h1, uint64_t h2) +static inline uint64_t update_h2(uint64_t k, uint64_t h1, uint64_t h2) { k ^= h2; k = ROT(k, 31);