Re: [PATCH] MNG detection fails with libjpeg-turbo

On 2017-11-30 23:21 -0500, The Wanderer wrote:
> On 2017-11-30 at 16:23, Alexander Strasser wrote:
[...]
> >>  yasm_check() {
> >>    echo >> "$TMPLOG"
> >>    cat "$TMPS" >> "$TMPLOG"
> >> @@ -5303,7 +5313,7 @@
> >>  if test "$_mng" = auto ; then
> >>    _mng=no
> >>    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
> >> +    return_statement_check_broken stdio.h libmng.h 'const char * p_ver = mng_version_text()' '!p_ver || p_ver[0] == 0' $mnglibs && _mng=yes
> > 
> > Would you be OK with omitting the function defined in the previous hunk
> > and changing this to use statement_check_broken ?
> 
> I would have no objection to it, as long as we understand what it's
> doing and what the consequences will be.
> 
> My primary concern is: why was this put there in the first place?
> Presumably it served some purpose at some point...

I did some digging... it was like that since it was added in 2008. The
configure check for libmng was initially added for the mng demuxer.

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 guess the original version was largely inspired by the png configure
check of that time. I didn't look how that one came into place though.


> > I have done the changes locally and everything seems to work as well.
> > 
> > I don't think the return part adds any value, because the built
> > executable is never run and thus its exit status isn't evaluated.
> > 
> > If it was added because of fear that linking could be spared because
> > of some compiler optimizations, I don't think it will happen in this
> > case. Also lots of other checks don't use the return_statement_check.
> > So if there is a problem, it would better be fixed for all checks.
> 
> 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?


> > LGTM otherwise.
> > 
> > I intent to commit my local version with the modified configure check
> > if I hear no objections.
> > 
> > Thank you for looking into this. I am not using vo mng and I didn't
> > notice it won't build on many systems.

After doing some code archaeology, I still intent to apply my local
version.

It will be a few days before I get back to it. So there is some time
voice disagreement or provide additional arguments. I think this wasn't
working for so long, so we can just take a little time.


  Alexander

[...]
_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng