[MPlayer-dev-eng] [PATCH] MNG detection fails with libjpeg-turbo

[Alexander Strasser]

Hi!

Sorry for the delay, I fear this discussion isn't over yet :)

On 2017-12-01 19:23 -0500, The Wanderer wrote:
> On 2017-12-01 at 18:11, Alexander Strasser wrote:
> > 
> > Then I looked at the actual mail thread, where the patch was
> > discussed. It was first submitted with a configrue check that
> > executed the compiled binary. That was questioned and later deemed
> > not acceptable because it would always make the test fail when
> > cross-compiling:
> > 
> > http://mplayerhq.hu/pipermail/mplayer-dev-eng/2008-September/058673.html
> 
> I haven't taken the time to read the discussion now, but based on that
> description, I think I may even vaguely remember its occurring.
> 
> >> I'd be leery of dropping code whose intended purpose we don't know
> >> or understand, but if it works equally well without that and we
> >> make sure that our own reasoning is findable in case someone does
> >> turn up a problem which could be the reason later on, then I have
> >> no strong objection.
> > 
> > With above explanation added to the commit message, that would
> > explain our reasoning, right?
> 
> It would be even better than what I'd intended, which was just to
> explain "since this is never run, we don't see how it can be useful, so
> we're dropping it rather than make other parts of the code more complex;
> however, we don't know why this was added, so this may need to be
> revisited in the future".
> 
> Since you've tracked down the reason why it was added, no such "we don't
> know" caveat is needed, and I don't see any reason not to drop that
> return statement. IMO it should probably have been removed along with
> the removal of "execute the compile-check binary".
> 
> > After doing some code archaeology, I still intent to apply my local 
> > version.
> 
> I approve this.

  I was about to commit the patch with the dropped return as
I have written above, but then I got into thinking about the
whole thing again.

  The libjpeg part of this puzzle was what got me into thinking
again. Regardless of our preference, it is legitimate for libjpeg
to document and say it needs inclusion of stdio.h before its
own headers.

  But in this discussion we are talking about libmng, where I find
it unacceptable to *need* inclusion of stdio.h *or not* depending
on a compile-time option of the lib.

  There are only 2 choices for libmng, document that it is always
needed to include stdio.h before libmng.h or to include it themselves.
And indeed I think they decided to do the latter. Though for some
unknown reason, this was removed from the Debian package in a seemingly
unrelated custom patch that is applied to libmng in the build process
of the libmng debian package:

  https://sources.debian.org/src/libmng/1.0.10+dfsg-3.1/debian/patches/support-lcms2.patch/

  So I believe MPlayer mng support is working fine when using vanilla
libmng or maybe even on all distributions not re-using the debian
package.

  This is why I do not really like to apply the patch anymore :(
It is like programming for the Debian version of libmng, when they
are actually the ones that got it wrong. This should be reported
to a person or a mailing list at Debian.

  Anyway I have attached a completely new patch that happens
to workaround the problems on Debian on the assumption that
we are never using JNG features in MPlayer anyway. In case you
wonder: I implemented it with a return to avoid compiler warnings
about not using variables at all. It was easy and cheap when
writing the whole C file anyway.


  Alexander

P.S.
Kind of unrelated to the issue at hand, we should probably fix that
SVN r35706 forgot to early break out of the loop if the compile check
succeeded. Shouldn't be difficult, I can do this myself while I am at it.
-------------- next part --------------
>From 97100e6036f484ef6ded7330417b0c7bccfd4f3a Mon Sep 17 00:00:00 2001
From: Alexander Strasser <eclipse7 at gmx.net>
Date: Sun, 17 Dec 2017 20:51:46 +0100
Subject: [PATCH] vo_mng: Define that we do not use libmng's JNG features

By chance this helps compilation on current Debian, where an
include of stdio.h was patched out of the libmng headers.
---
 configure      | 10 +++++++++-
 libvo/vo_mng.c |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 757ee20a7..90d8f5c51 100755
--- a/configure
+++ b/configure
@@ -5302,8 +5302,16 @@ fi
 echocheck "MNG support"
 if test "$_mng" = auto ; then
   _mng=no
+  cat > $TMPC << EOF
+#define MNG_NO_INCLUDE_JNG
+#include <libmng.h>
+int main(void) {
+  const char * p_ver = mng_version_text();
+  return !p_ver;
+}
+EOF
   for mnglibs in '-lmng -lz' '-lmng -ljpeg -lz' ; do
-    return_statement_check libmng.h 'const char * p_ver = mng_version_text()' '!p_ver || p_ver[0] == 0' $mnglibs && _mng=yes
+    cc_check $mnglibs && _mng=yes
   done
 fi
 echores "$_mng"
diff --git a/libvo/vo_mng.c b/libvo/vo_mng.c
index e98163fbe..a23cdd847 100644
--- a/libvo/vo_mng.c
+++ b/libvo/vo_mng.c
@@ -30,6 +30,7 @@
 
 #include <zlib.h>
 
+#define MNG_NO_INCLUDE_JNG
 #define MNG_INCLUDE_WRITE_PROCS
 #define MNG_ACCESS_CHUNKS
 #define MNG_SUPPORT_READ
-- 
2.15.0