Opened 15 years ago

Closed 14 years ago

#19142 closed defect (fixed)

faac doesn't support mp4

Reported by: anddam (Andrea D'Amore) Owned by: pguyot (Paul Guyot)
Priority: Normal Milestone:
Component: ports Version: 1.7.0
Keywords: faac mp4 support Cc: jeremyhu (Jeremy Huddleston Sequoia), tristan@…, milosh@…, bytestorm@…, dbevans (David B. Evans), 0xced (Cédric Luthi), rmsfisher@…
Port: faac

Description

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

Attachments (4)

Portfile (1.2 KB) - added by anddam@… 15 years ago.
configure.in-patch (607 bytes) - added by anddam@… 15 years ago.
build output.txt (41.2 KB) - added by anddam@… 15 years ago.
faac-Portfile.diff (501 bytes) - added by rmsfisher@… 14 years ago.

Download all attachments as: .zip

Change History (19)

Changed 15 years ago by anddam@…

Attachment: Portfile added

Changed 15 years ago by anddam@…

Attachment: configure.in-patch added

Changed 15 years ago by anddam@…

Attachment: build output.txt added

comment:1 Changed 15 years ago by dbevans (David B. Evans)

Cc: pguyot@… tristan@… milosh@… bytestorm@… added

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.

comment:2 Changed 15 years ago by dbevans (David B. Evans)

Cc: pguyot@… removed

comment:3 Changed 15 years ago by dbevans (David B. Evans)

Cc: devans@… added

Cc Me!

comment:4 in reply to:  1 Changed 15 years ago by 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.

comment:5 Changed 15 years ago by 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.

comment:6 Changed 15 years ago by 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.

comment:7 Changed 15 years ago by 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 .

comment:8 in reply to:  5 Changed 15 years ago by 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?

comment:9 Changed 15 years ago by (none)

Milestone: Port Bugs

Milestone Port Bugs deleted

comment:10 Changed 15 years ago by 0xced (Cédric Luthi)

Cc: cedric.luthi@… added

Cc Me!

comment:11 Changed 15 years ago by 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.

comment:12 Changed 14 years ago by rmsfisher@…

Cc: rmsfisher@… added

Cc Me!

comment:13 Changed 14 years ago by 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.

Changed 14 years ago by rmsfisher@…

Attachment: faac-Portfile.diff added

comment:14 Changed 14 years ago by rmsfisher@…

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

comment:15 Changed 14 years ago by rmsfisher@…

Resolution: fixed
Status: newclosed

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

Note: See TracTickets for help on using tickets.