[Ffmpeg-devel] [bug] raw.c link failure on ff_ac3_parse_header

[Måns Rullgård]

Diego Biurrun <diego at biurrun.de> writes:

> On Mon, Apr 09, 2007 at 09:42:07PM +0100, M?ns Rullg?rd wrote:
>> Diego Biurrun <diego at biurrun.de> writes:
>> 
>> > On Thu, Mar 22, 2007 at 10:35:40AM +0200, Uoti Urpala wrote:
>> >> On Thu, 2007-03-22 at 09:21 +0100, Diego Biurrun wrote:
>> >> > On Wed, Mar 21, 2007 at 11:31:34PM -0500, Justin Ruggles wrote:
>> >> > > It seems to me that putting ac3.o in the OBJS list in the libavcodec
>> >> > > Makefile would solve this.  Would this be an ok thing to do?
>> >> > 
>> >> > No.  The report remains incomplete and the problem unreproducible.
>> >> 
>> >> The problem seems clear: libavformat/raw.c unconditionally compiles a
>> >> call to ff_ac3_parse_header, but that function itself is not compiled
>> >> unconditionally.
>> >
>> > Attached is a patch that fixes the issue and makes ff_ac3_parse_header
>> > conditional to CONFIG_AC3_PARSER, which I believe is the correct
>> > solution.  Is it necessary to #ifdef ff_ac3_parse header in ac3.h as
>> > well?
>> 
>> If it compiles correctly in both cases, no.
>
> It compiles fine.  It was more of a general style question.  My question
> was whether it should be indicated with an #ifdef in the header file
> that this function is conditionally compiled.

I generally use the simple rule that code with no effect shouldn't
exist.  This applies equally to preprocessor directives and actual
code.

>> > --- libavformat/allformats.c	(revision 8688)
>> > +++ libavformat/allformats.c	(working copy)
>> > @@ -45,7 +45,8 @@
>> >      avcodec_register_all();
>> >  
>> >      REGISTER_DEMUXER (AAC, aac);
>> > -    REGISTER_MUXDEMUX(AC3, ac3);
>> > +    if (ENABLE_AC3_PARSER) REGISTER_DEMUXER (AC3, ac3);
>> > +    REGISTER_MUXER   (AC3, ac3);
>> 
>> You could make AC3_DEMUXER depend on AC3_PARSER in configure instead.
>> That might be cleaner.
>
> Yes, I might do that.
>
> I'm thinking that it might be cleaner to move ff_ac3_parse_header out of
> ac3.c and into parser.c where the other parse functions are.  This would
> save some #ifdefs in ac3.c.

I'm undecided on the location of these things.  In a way I like to
keep everything relating to a particular codec together, especially
when the decoder uses the same functions.  OTOH keeping the parser
code in one place has its virtues.

-- 
M?ns Rullg?rd
mans at mansr.com