faac doesn't support mp4

[anddam (Andrea D'Amore)]

I noticed that faac from ports tree doesn't support mp4.

it appears that these lines are at odds with one another:

23	configure.args  --without-mp4v2
24	
25	depends_lib     port:libmp4v2

Anyway after upgrading libmp4v2 as per #19141 I had to patch configure.in in order to let it see external libmp4v2 (internal one appears to be very old).

Even if configure successfully detects libmp4v2

checking whether MP4Create is declared... yes
checking for MP4MetadataDelete in -lmp4v2... yes
checking whether MP4MetadataDelete is declared... yes
checking for MP4MetadataDelete in -lmp4v2... (cached) yes
configure: *** Building with external mp4v2 ***

the resulting executable still reports

$ `port dir faac`/work/faac-1.28/frontend/faac --help 2>&1 | grep MP4
MP4 specific options:
  MP4 support unavailable.

Can anyone reports a faac build with mp4 support?

I'm attaching Portfile, patchfile and the build output for faac@1.28_2+mp4v2

[dbevans (David B. Evans)]

It's a bit confusing but the configuration option

--with-mp4v2            compile libmp4v2

means to build the embedded libmp4v2 source and without means to look for an external library which it does by looking in mp4.h etc.

This is correct for the current MacPorts version of libmp4v2 and faac configures as follows

checking whether MP4Create is declared... yes
checking for MP4MetadataDelete in -lmp4v2... yes
checking whether MP4MetadataDelete is declared... yes
checking for MP4MetadataDelete in -lmp4v2... (cached) yes
configure: *** Building with external mp4v2 ***

The submitted patch addresses changes necessary to build against an upgraded version of libmp4v2 (not yet committed) as proposed in #19141 which is not compatible with the current version.

I believe the mp4v2 variant proposed here is unnecessary (mp4 support should be the default) and incorrectly handles configuration using the external library.

Patches would continue to be necessary to use the proposed upgraded library because of the change from mp4.h to mp4v2.h. However, a better patch for configure would be to look for either mp4.h or mp4v2.h and configure appropriately.

I would be interested in whether/how the upstream developers intend to support this new version of libmp4v2. As packagers we should probably follow their lead.

[dbevans (David B. Evans)]

Cc Me!

[anddam@…]

Replying to devans@…:

--with-mp4v2 means to build the embedded libmp4v2 source and without means to look for an external library

Are you sure? Check ​this line from my output, configure script recognizes external library even if --with-mp4v2 ​was provided to configure.

The submitted patch addresses changes necessary to build against an upgraded version of libmp4v2 (not yet committed) as proposed in #19141 which is not compatible with the current version.

I believe the mp4v2 variant proposed here is unnecessary (mp4 support should be the default) and incorrectly handles configuration using the external library.

I felt it more natural this way, if it is an option in the original package should be a variant. easytag and easytag-devel have mp4 variant too.

Patches would continue to be necessary to use the proposed upgraded library because of the change from mp4.h to mp4v2.h. However, a better patch for configure would be to look for either mp4.h or mp4v2.h and configure appropriately.

I'm not sure what you mean, how would you do that?

I would be interested in whether/how the upstream developers intend to support this new version of libmp4v2. As packagers we should probably follow their lead.

I'm dropping a line to them, the project's activity is quiet, at least. Considering that according to files in source tree they are embedding an mp4v2 release from 2004, I think they are not interested in bleeding edge.

[dbevans (David B. Evans)]

Good idea about dropping a line.

Concerning --with-mp4v2, upon review, its a little of both ;-)

Here are the lines from configure.in

AM_CONDITIONAL(WITH_MP4V2, false)

AC_CHECK_DECLS([MP4Create, MP4MetadataDelete],
               AC_CHECK_LIB(mp4v2, MP4MetadataDelete, external_mp4v2=yes,
                            external_mp4v2=no, -lstdc++),
               external_mp4v2=no, [#include <mp4.h>])

if test x$external_mp4v2 = xyes; then
  AC_MSG_NOTICE([*** Building with external mp4v2 ***])
else
  if test x$WITHMP4V2 = xyes; then
     AC_MSG_NOTICE([*** Building with internal mp4v2 ***])
     AM_CONDITIONAL(WITH_MP4V2, true)
     AC_CONFIG_LINKS(common/mp4v2/mpeg4ip_config.h:config.h)
     MY_DEFINE(HAVE_LIBMP4V2)
  else
     AC_MSG_NOTICE([*** Building WITHOUT mp4v2 ***])
  fi
fi

So you can see that if --with-mp4v2 is set it tries to build from the internal source but only if an external library is not found. From our point of view this option should make no difference at all, since the external library exists. So the question remains whether this actually links in the external library or not.

Anyway, should resolve the issue with the current libmp4v2 first and then worry about the new version. Please double check your results against the current libmp4v2 port and I'll do the same.

If you file a ticket/report upstream please reference it here.

[anddam@…]

I see, actually I'm surprised my build went through as I just patched the configure.in but forgot to patch the source files, so Makefile refers to an external library while actual sources are still including <mp4.h> . Only following should be patched as patching the internal mp4v2 is pointless:

common/Cfaac/Cfaac.h
common/Cfaac/Cfaad.h
common/Cfaac/CTag.c
common/Cfaac/CTag.h
frontend/main.c

As reference I sent a mail to the developer at info at audiocoding dot com.

[anddam@…]

I received a reply:

I looked into this, but it doesn't seem very trivial, they changed a lot of functions I use.
I'm not sure if I have time for fixing this anytime soon.

so for now it's better to keep libmp4v2 as it is now and make faac works with it, i.e. getting MP4 support in actual binary.

We could create a new port for libmp4v2 like libmp4v2-2 or mp4v2 as it is the actual project name. Or we could just leave it at 1.5 as there's no point in upgrading a library that noone is using anyway, but this kind of discussion should rather be done in #19141 .

[anddam@…]

Replying to devans@…:

Anyway, should resolve the issue with the current libmp4v2 first and then worry about the new version. Please double check your results against the current libmp4v2 port and I'll do the same.

I reverted libmp4v2 to @1.5.0.1_0 and rebuilt faac, still I get

MP4 specific options:
  MP4 support unavailable.

Does your build support MPEG-4?

[(none)]

Milestone Port Bugs deleted

[0xced (Cédric Luthi)]

Cc Me!

[0xced (Cédric Luthi)]

I wrote it in #19141 (comment 7), but I'll report it here for completeness: mp4 support is not built into faac is because the configure.in file is bugged: it does not do MY_DEFINE(HAVE_LIBMP4V2) when Building with external mp4v2 which result in no mp4 support at all.

[rmsfisher@…]

Cc Me!

[rmsfisher@…]

The source for faac has embedded a copy of source for libmp4.
See for yourselves, extract the source and look in faac-1.28/common/mp4v2.
Because of this, I think we should do away with both of the following lines:

configure.args  --without-mp4v2 
depends_lib     port:mp4v2

This change would allow this port to support mp4 functions and resolve its seemingly-broken dependency on (lib)mp4v2.

[rmsfisher@…]

Unless y'all object I will apply the above patch this weekend and close the ticket.

[rmsfisher@…]

Patch applied in r67546. It took a little longer than the "this weekend" I promised in the above reply months ago.