On Fri, 23 Oct 2015 10:14:24 +0200
Andreas Enge <andreas@enge.fr> wrote:

> Hello,
> 
> On Tue, Oct 20, 2015 at 04:56:55PM +0300, Efraim Flashner wrote:
> > I saw that mplayer 1.2 came out recently. I wanted to make sure that the patch was good as is, and shouldn't be split. Since the last release, libdvdnav and libdvdcss are no longer packaged, so I added them to the inputs. Also, I found the config flag to use the shared ffmpeg library, so I added that in, and I took out the depreciated config flag that was there before.
> > mplayer 1.2 will build with just the flag removed, but it seemed to me that we didn't want to use the bundled ffmpeg so I tossed it and the two libdvds in.  
> 
> this is an overall useful patch! I executed
>    $ guix gc --references /gnu/store/wadr15hfyvcn4zg2zj2ygqb8ypkzsg48-mplayer-1.2
> and indeed libvorbis is referenced, so the flag you removed is apparently
> not needed any more.
my first hint was the compile error it gave when I first tried to leave it in :)

> 
> However, there is no trace of libdvd*. Looking at the build log on hydra:
>    http://hydra.gnu.org/build/756649/log/raw
> I find:
> Checking for DVD support (libdvdnav) ... no 
> Could you have a look at what went wrong, and whether we need a configuration
> flag?
we shouldn't need to have one:

efraim@debian-netbook:~/mplayer-export-2015-10-23$ grep dvd configure
  --disable-dvdnav       disable libdvdnav [autodetect]
  --disable-dvdread      disable libdvdread [autodetect]

but later in the guts of configure ...

echocheck "dvdread"
if test "$_dvdread" = auto ; then
  _dvdread=no
    _dvdreadcflags=$($_dvdreadconfig --cflags 2> /dev/null)
    _dvdreadlibs=$($_dvdreadconfig --libs 2> /dev/null)
    if statement_check dvdread/dvd_reader.h 'DVDOpen("test")' $_dvdreadcflags $_dvdreadlibs ; then
      _dvdread=yes
      extra_cflags="$extra_cflags $_dvdreadcflags"
      extra_ldflags="$extra_ldflags $_dvdreadlibs"
    fi
fi

if the spacing on "_dvdread=no" is a problem then it skips the next few lines
and never actually checks for dvdread/dvd_reader.h. (todo: create an
alternate, fixed version to test theory, and pass patch upstream if necessary)

it turns out there are lots of --enable/--disable flags, for just about
everything, so i'll pass it --enable-dvdread --enable-dvdnav and see how that
works.

> 
> Maybe it would also be good to remove the bundled ffmpeg copy from the
> source, using a snippet.

the flag "--disable-ffmpeg_a" does work for disabling the bundled ffmpeg; the
first time I built it I hadn't added the flag, and the build time was ~40
minutes instead of ~4. I think I just had poor/ambiguous phrasing in my email.

> 
> Is there any other bundled software?
looking in the source directory I see folders for libaf, libao2, libass
(which I'm still sure I'm reading wrong), libmenu, libmcodecs, libmpdemux,
libmpeg2 and libvo.

from looking through the configure more, I found this snippet:

        if cc_check -lass ; then
            res_comment="external"
            extra_ldflags="$extra_ldflags -lass"
        elif test "$ass_internal" = auto ; then
            ass_internal=yes
        else
            _ass=no
        fi

so it looks like there's still a considerable amount of work left to check
for these and potentially other libraries. Looking at the source code quickly
I don't see an ffmpeg folder dumped somewhere so I think I'm going to need to
read configure closely to see what else it tries to bundle.

Bundling libraries makes the compiled binary larger, so I think I'll be
building this a bunch more times taking out all the bundled libraries.

I'll take another look at the configure file (wc -l configure=9501!!), but it
is pretty big so I'll attach it also for that whole more eyes thing. Its from
the most recent source snapshot, but its also only about 2 weeks after
release, so it should be close enough to the one we're using.


ACTUALLY, this is a big enough project that I've downloaded the 1.2 release
to work against, and I see the ffmpeg folder now. I've included that
configure instead of the snapshot one, and I think that's the only big
difference. I'd go back and change the email to reflect that now I'm working
on the correct snapshot (the release "snapshot"), but that's how I normally
make my emails incomprehensible. :)

> 
> Thanks for your work!
> 
> Andreas
> 

Looking at the todo list, I feel like it was just part 1 to update the
version number, but that we're probably not worse off with bundled libraries
than we were before.


-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted