[FFmpeg-devel] [RFC] Split libavformat

[Diego Biurrun]

On Tue, Nov 20, 2007 at 08:37:09AM +0100, Luca Abeni wrote:
>
> as promised, here is my attempt at splitting libavformat.

I'm happy to see you follow through on your promise.  Here's my review:

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ffmpeg/libavdevice/Makefile	2007-11-19 19:57:22.061554240 +0100
> @@ -0,0 +1,36 @@
> +#
> +# libavdevice Makefile
> +# (c) 2000-2003, 2007 Fabrice Bellard
> +#

Fabrice did not write this.  In fact, having him listed as copyright
holder is doubtful in the other Makefiles as well, Mans and I rewrote
almost every single line.

> +CFLAGS += -I$(SRC_PATH)/libavcodec -I$(SRC_PATH)/libavformat

Are both really needed?

> +# muxers/demuxers
> +OBJS-$(CONFIG_BKTR_DEMUXER)              += bktr.o

Is there a better name than (de)muxers for this?

> +EXTRALIBS := -L$(BUILD_ROOT)/libavutil -lavutil$(BUILDSUF) \
> +             -lavcodec$(BUILDSUF) -L$(BUILD_ROOT)/libavcodec \
> +             -lavformat$(BUILDSUF) -L$(BUILD_ROOT)/libavformat $(EXTRALIBS)

Is all of this really needed?

> +NAME=avdevice
> +LIBVERSION=$(LAVFVERSION)
> +LIBMAJOR=$(LAVFMAJOR)

I think I prefer these lines at the top of the Makefile, but yes, this
is inconsistently handled in the other Makefiles.

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ffmpeg/libavdevice/allformats.c	2007-11-19 19:57:22.062554088 +0100
> @@ -0,0 +1,57 @@
> +/*
> + * Register all the grabbing formats 
> + * Copyright (c) 2000, 2001, 2002, 2007 Fabrice Bellard

Fabrice did not write this.

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ffmpeg/libavdevice/avdevice.h	2007-11-19 19:57:22.063553936 +0100
> @@ -0,0 +1,33 @@
> +/*
> + * copyright (c) 2001, 2007 Fabrice Bellard

ditto

> +#ifndef FFMPEG_AVDEV_H
> +#define FFMPEG_AVDEV_H
> +
> +#endif /* FFMPEG_AVDEV_H */

Please use the filename as multiple inclusion guard.

> --- ffmpeg.orig/configure	2007-11-19 19:49:07.299769464 +0100
> +++ ffmpeg/configure	2007-11-19 19:57:22.066553480 +0100
> @@ -920,6 +920,8 @@
>  BSF_LIST=`sed -n 's/^[^#]*BSF.*(.*, *\(.*\)).*/\1_bsf/p' "$source_path/libavcodec/allcodecs.c"`
>  MUXER_LIST=`sed -n 's/^[^#]*_MUX.*(.*, *\(.*\)).*/\1_muxer/p' "$source_path/libavformat/allformats.c"`
>  DEMUXER_LIST=`sed -n 's/^[^#]*DEMUX.*(.*, *\(.*\)).*/\1_demuxer/p' "$source_path/libavformat/allformats.c"`
> +MUXER_LIST="$MUXER_LIST `sed -n 's/^[^#]*_MUX.*(.*, *\(.*\)).*/\1_muxer/p' "$source_path/libavdevice/allformats.c"`"
> +DEMUXER_LIST="$DEMUXER_LIST `sed -n 's/^[^#]*DEMUX.*(.*, *\(.*\)).*/\1_demuxer/p' "$source_path/libavdevice/allformats.c"`"

I'd rather see this as a new list.

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ffmpeg/libavnet/Makefile	2007-11-19 20:09:18.795594104 +0100

All my comments from above apply here as well.

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ffmpeg/libavnet/allformats.c	2007-11-19 20:09:18.795594104 +0100

ditto

> --- ffmpeg.orig/configure	2007-11-19 19:57:22.066553480 +0100
> +++ ffmpeg/configure	2007-11-19 20:09:18.798593648 +0100
> @@ -922,7 +922,10 @@
>  DEMUXER_LIST=`sed -n 's/^[^#]*DEMUX.*(.*, *\(.*\)).*/\1_demuxer/p' "$source_path/libavformat/allformats.c"`
>  MUXER_LIST="$MUXER_LIST `sed -n 's/^[^#]*_MUX.*(.*, *\(.*\)).*/\1_muxer/p' "$source_path/libavdevice/allformats.c"`"
>  DEMUXER_LIST="$DEMUXER_LIST `sed -n 's/^[^#]*DEMUX.*(.*, *\(.*\)).*/\1_demuxer/p' "$source_path/libavdevice/allformats.c"`"
> +MUXER_LIST="$MUXER_LIST `sed -n 's/^[^#]*_MUX.*(.*, *\(.*\)).*/\1_muxer/p' "$source_path/libavnet/allformats.c"`"
> +DEMUXER_LIST="$DEMUXER_LIST `sed -n 's/^[^#]*DEMUX.*(.*, *\(.*\)).*/\1_demuxer/p' "$source_path/libavnet/allformats.c"`"

see above

> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ ffmpeg/libavnet/avnet.h	2007-11-19 20:09:18.805592584 +0100
> @@ -0,0 +1,49 @@
> +/*
> + * copyright (c) 2001, 2007 Fabrice Bellard

.. and once more ..

Diego