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

The Wanderer <[email protected]> Sun, 17 Dec 2017 19:20:24 -0500
Newsgroups	gmane.comp.video.mplayer.devel
Organization	This space intentionally left blank.
Message-ID	<[email protected]>
On 2017-12-17 at 16:55, Alexander Strasser wrote:

> 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:

>> > 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.

I hadn't even considered that aspect, but you're right.

> 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.

Apparently it's already been reported, in 2015, as Debian bug #798781.

That bug was closed as "fixed in new upstream version" back in 2016, but
the new version in question is apparently *still* in experimental
(rather than unstable) after all this time.

I'm not sure offhand of where to look for a reason for that, but as I'm
about to come up on Christmas break from my workplace, I'll see about
making contact with someone on the Debian side in the next week or so to
inquire about the situation. (Though the holiday season may mean they're
not as quick to respond...)

>   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.

It's not as clean a solution, but it looks good to me. I haven't tested
it yet, since I don't have an MPlayer rebuild pending at the moment,
though I can if that is necessary. (I tend to do a rebuild every
couple-three weeks when a new library version comes through the Debian
repositories and breaks runtime linking on my existing build.)

> 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.

Yeah, I missed spotting that myself. Worth doing.

-- 
   The Wanderer

The reasonable man adapts himself to the world; the unreasonable one
persists in trying to adapt the world to himself. Therefore all
progress depends on the unreasonable man.         -- George Bernard Shaw

_______________________________________________
MPlayer-dev-eng mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
signature.asc (application/pgp-signature, 833 B)