| Message ID | CAB0OVGr7Y9-U4+aW1p4FXq3KnpKBiLG4LrUUvG7yAP=-Z=9NBg@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > New patch attached. > +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is supposed > +to work out-of-the-box. Is this true or is it just working by accident? Carl Eugen
Hello, On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > >> New patch attached. > >> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is supposed >> +to work out-of-the-box. > > Is this true or is it just working by accident? AMD64 dictates that float operations must occur with scalar SSE operations, so it's not working by accident. It works because float operations don't use x87 stack (which is shared with MMX) Best regards, Guillaume
Hi! 2016-10-03 13:05 GMT+02:00 Guillaume POIRIER <poirierg@gmail.com>: > Hello, > > On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >> 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: >> >>> New patch attached. >> >>> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is supposed >>> +to work out-of-the-box. >> >> Is this true or is it just working by accident? > > AMD64 dictates that float operations must occur with scalar SSE > operations, so it's not working by accident. It works because float > operations don't use x87 stack (which is shared with MMX) Thank you for the explanation! Ping on the patch: We will not guard every asm call with emms_c() and Rich will not change his alloc() implementation for us: Can't we just document that it doesn't work and return to bugs that we can fix? Or should we consider to guard the three calls to mallc(), realloc() and free()? This works here as expected with 32bit musl but likely does not fix the underlying issue. Carl Eugen
On Mon, Oct 3, 2016 at 1:37 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Hi! > > 2016-10-03 13:05 GMT+02:00 Guillaume POIRIER <poirierg@gmail.com>: >> Hello, >> >> On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: >>> 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: >>> >>>> New patch attached. >>> >>>> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is supposed >>>> +to work out-of-the-box. >>> >>> Is this true or is it just working by accident? >> >> AMD64 dictates that float operations must occur with scalar SSE >> operations, so it's not working by accident. It works because float >> operations don't use x87 stack (which is shared with MMX) > > Thank you for the explanation! > > Ping on the patch: > We will not guard every asm call with emms_c() and Rich will not > change his alloc() implementation for us: Can't we just document > that it doesn't work and return to bugs that we can fix? > > Or should we consider to guard the three calls to mallc(), realloc() > and free()? > This works here as expected with 32bit musl but likely does not > fix the underlying issue. > There is no reason to call anything unsupported because FFmpeg violates the C calling convention, nor is there a reason to add overhead to every single call. The underlying problem is that mmx code is mixed with allocations, which seems like an unusual case to begin with since allocations should not be part of performance critical loops or something like that. So, the appropriate answer is to find the code in question that causes these issues, and just add an emms there, or re-structure it to take the allocations out of the mmx code. Anything else just masks potential issues. - Hendrik
Hi, On Mon, Oct 3, 2016 at 7:37 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > Hi! > > 2016-10-03 13:05 GMT+02:00 Guillaume POIRIER <poirierg@gmail.com>: > > Hello, > > > > On Mon, Oct 3, 2016 at 11:38 AM, Carl Eugen Hoyos <ceffmpeg@gmail.com> > wrote: > >> 2016-10-03 3:52 GMT+02:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>: > >> > >>> New patch attached. > >> > >>> +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is > supposed > >>> +to work out-of-the-box. > >> > >> Is this true or is it just working by accident? > > > > AMD64 dictates that float operations must occur with scalar SSE > > operations, so it's not working by accident. It works because float > > operations don't use x87 stack (which is shared with MMX) > > Thank you for the explanation! > > Ping on the patch: The patch is unlikely to assist in fixing the issue and is likely to cause further inflammation. Therefore I would prefer you did not apply the patch and also please don't send a new version that is differently worded. I would prefer to work with upstream (musl) and fix the issue where ffmpeg running with musl crashes. Whether that means changing ffmpeg or musl remains to be seen. Let's chat with the developers and figure something out. Ronald
On Mon, Oct 03, 2016 at 08:01:05AM -0400, Ronald S. Bultje wrote: > > Ping on the patch: > The patch is unlikely to assist in fixing the issue and is likely to cause > further inflammation. Therefore I would prefer you did not apply the patch > and also please don't send a new version that is differently worded. > > I would prefer to work with upstream (musl) and fix the issue where ffmpeg > running with musl crashes. Whether that means changing ffmpeg or musl > remains to be seen. Let's chat with the developers and figure something out. The standard C library API is what it is called, a standard. What you are talking about is to ask Rich to modify musl to hide ffmpeg's non-compliance bug (which glibc/uclibc do by sheer luck but this may change any time). With all the competence present here, is it really infeasible to improve the code structure so that it does not involve the C library in the bit-crunching performance critical paths?? Rune
Hi Rune, On Mon, Oct 3, 2016 at 10:26 AM, <u-9iep@aetey.se> wrote: > On Mon, Oct 03, 2016 at 08:01:05AM -0400, Ronald S. Bultje wrote: > > > Ping on the patch: > > > The patch is unlikely to assist in fixing the issue and is likely to > cause > > further inflammation. Therefore I would prefer you did not apply the > patch > > and also please don't send a new version that is differently worded. > > > > I would prefer to work with upstream (musl) and fix the issue where > ffmpeg > > running with musl crashes. Whether that means changing ffmpeg or musl > > remains to be seen. Let's chat with the developers and figure something > out. > > The standard C library API is what it is called, a standard. > > What you are talking about is to ask Rich to modify musl to hide ffmpeg's > non-compliance bug (which glibc/uclibc do by sheer luck but this may change > any time). > > With all the competence present here, is it really infeasible to improve > the code structure so that it does not involve the C library in the > bit-crunching performance critical paths?? I'm sure you understand that screaming around like a madman is unlikely to improve anything. - I don't want to litter the code with emms calls all around calls to libc functions, certainly not every en/decoder, this would have to be in generic code only; - calling emms_c before calling user callbacks or malloc/free calls is potentially doable, but doesn't make us abide to the standard in a literal sense. We also need to go over the code and see how many changes this requires; - or we can just do what we do now and work with musl people to change their code. If it turns out the first two are undoable and musl devs are unwilling to do this, then we'll have to reconsider Carl's patch and call musl on x86-32 unsupported, with a link to the relevant discussion to back up our reasoning (to prevent bystanders from calling us trolls). Again, this requires some time and thought. Ronald
On Mon, Oct 03, 2016 at 04:26:50PM +0200, u-9iep@aetey.se wrote: > With all the competence present here, is it really infeasible to improve > the code structure so that it does not involve the C library in the > bit-crunching performance critical paths?? Bad wording. Should be: "assembler-implemented" paths. There is nothing wrong with using C library's routines - they are well optimized. But if they are relied upon, they have to be respected, they assume namely that the caller behaves according to the contract. If you have a real need to do things which are incompatible with the API contract, then the C library simply is not available. Rune
Ronald, I sincerely appreciate that you are taking the effort to talk to me and explain your position. Unfortunately in your messages you consistently ignored a crucial point and this is the last time I try to retell it: > On Mon, Oct 3, 2016 at 10:26 AM, <u-9iep@aetey.se> wrote: > > What you are talking about is to ask Rich to modify musl to hide ffmpeg's > > non-compliance bug (which glibc/uclibc do by sheer luck but this may change > > any time). Which you answer with > - or we can just do what we do now and work with musl people to change > their code. Once again, musl has absolutely no reason to change its code, their code is well done and fully compliant. Their responsibility is rather to the contrary, _not_ to make changes which would hide sneaky bugs in applications. UB is a quite hideous bug. > If it turns out the first two are undoable and musl devs are > unwilling to do this, then we'll have to reconsider Carl's patch and call > musl on x86-32 unsupported, with a link to the relevant discussion to back You can not just "call musl on x86-32 unsupported", the only honest option is to document that ffmpeg is not compliant/safe. The problem is not in musl. Last time in this discussion let me try to make this fact understood: Musl merely showed you the problem and now you are suggesting to "document that the messenger with his bad news about our health is no longer welcome". > Again, this requires some time and thought. This sounds very reasonable. Best regards, Rune
On Mon, 3 Oct 2016 17:01:12 +0200 u-9iep@aetey.se wrote: > Ronald, > > I sincerely appreciate that you are taking the effort to talk to me > and explain your position. > > Unfortunately in your messages you consistently ignored a crucial point > and this is the last time I try to retell it: > > > On Mon, Oct 3, 2016 at 10:26 AM, <u-9iep@aetey.se> wrote: > > > What you are talking about is to ask Rich to modify musl to hide ffmpeg's > > > non-compliance bug (which glibc/uclibc do by sheer luck but this may change > > > any time). > > Which you answer with > > > - or we can just do what we do now and work with musl people to change > > their code. > > Once again, musl has absolutely no reason to change its code, > their code is well done and fully compliant. > > Their responsibility is rather to the contrary, _not_ to make changes > which would hide sneaky bugs in applications. UB is a quite hideous bug. > > > If it turns out the first two are undoable and musl devs are > > unwilling to do this, then we'll have to reconsider Carl's patch and call > > musl on x86-32 unsupported, with a link to the relevant discussion to back > > You can not just "call musl on x86-32 unsupported", the only honest option > is to document that ffmpeg is not compliant/safe. > > The problem is not in musl. > Last time in this discussion let me try to make this fact understood: > > Musl merely showed you the problem and now you are suggesting to "document > that the messenger with his bad news about our health is no longer welcome". musl could also choose to abandon its incredibly "clever" hack (that makes almost everyone who sees it go "WTF"), and, as was pointed our on IRC, probably increase the efficiency and readability of the code in question. Yes, musl is technically in the right, but only technically.
On Mon, Oct 03, 2016 at 05:27:19PM +0200, wm4 wrote: > > Musl merely showed you the problem and now you are suggesting to "document > > that the messenger with his bad news about our health is no longer welcome". > > musl could also choose to abandon its incredibly "clever" hack (that > makes almost everyone who sees it go "WTF"), and, as was pointed our on :-D IOW it exposes the _bugs_ in how people are accustomed to reason. > IRC, probably increase the efficiency and readability of the code in > question. Yes, musl is technically in the right, but only technically. Is your opinion (about efficiency) sufficiently well informed? No offence but I assume you to be a mutimedia guru, not a standard library expert, which the author of the code is. We shall not blindly accept authority of course. If you have a suggestion for improvement, given a reasonable ground it would be useful on the musl mailing list. Here and without a real ground it looks like a FUD. As for readability, it is your personal opinion. Again: no offence! Standard libraries are just a quite different area, it postulated other skills and presents other implementation challenges than multimedia programming. Friendly regards, Rune
On Mon, Oct 3, 2016 at 7:24 PM, <u-9iep@aetey.se> wrote: > On Mon, Oct 03, 2016 at 05:27:19PM +0200, wm4 wrote: >> > Musl merely showed you the problem and now you are suggesting to "document >> > that the messenger with his bad news about our health is no longer welcome". >> >> musl could also choose to abandon its incredibly "clever" hack (that >> makes almost everyone who sees it go "WTF"), and, as was pointed our on > > :-D > IOW it exposes the _bugs_ in how people are accustomed to reason. > >> IRC, probably increase the efficiency and readability of the code in >> question. Yes, musl is technically in the right, but only technically. > > Is your opinion (about efficiency) sufficiently well informed? > No offence but I assume you to be a mutimedia guru, not a standard > library expert, which the author of the code is. > > We shall not blindly accept authority of course. If you have a suggestion > for improvement, given a reasonable ground it would be useful on the musl > mailing list. > > Here and without a real ground it looks like a FUD. > > As for readability, it is your personal opinion. > > Again: no offence! Standard libraries are just a quite different area, > it postulated other skills and presents other implementation challenges > than multimedia programming. > Optimized code is the same everywhere, you just write different algorithms. - Hendrik
On Mon, Oct 03, 2016 at 07:28:08PM +0200, Hendrik Leppkes wrote: > > Again: no offence! Standard libraries are just a quite different area, > > it postulated other skills and presents other implementation challenges > > than multimedia programming. > > Optimized code is the same everywhere, you just write different algorithms. Note "the same everywhere". Optimization goals, constraints and strategies are different in different domains. No one is an expert in all of the various domains. Not me, not you. Yes it is hard to accept this, especially for very bright people who really know exceptionally many things - but "a lot" is not enough for everywhere. I hope we can stop discussing musl internals, there is no reason to do it on this list, nor other prerequisits for the discussion to make sense. As a concerned user of ffmpeg on 32-bit Intel I plead to make ffmpeg fully usable on this platform regardless of the internals of the C library being used. IOW I plead to make ffmpeg conform to the standards. My expectation and hope is that this can be done without making a noticeable performance sacrifice. Relying on C library internals is a hack, and not a robust one. Yours, Rune
From 008248a202e9eb6c1f946467a6fd45825a82a92a Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos <cehoyos@ag.or.at> Date: Mon, 3 Oct 2016 03:50:29 +0200 Subject: [PATCH] doc/platform: Mention musl where x86_32 is not supported. --- doc/platform.texi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/platform.texi b/doc/platform.texi index 21b135f..5c252ac 100644 --- a/doc/platform.texi +++ b/doc/platform.texi @@ -77,6 +77,14 @@ optimized assembly functions. @uref{http://www.finkproject.org/, Fink}, @uref{https://mxcl.github.com/homebrew/, Homebrew} or @uref{http://www.macports.org, MacPorts} can easily provide it. +@section musl + +Compilation with @uref{http://www.musl-libc.org/, musl} on x86-64 is supposed +to work out-of-the-box. +Do not compile FFmpeg with musl on x86_32 with assembler optimizations, random +crashes have to be expected because FFmpeg does not always clear the fpu state +as required by POSIX. + @chapter DOS -- 1.7.10.4