[FFmpeg-devel] [PATCH]Add Dirac support to ffmpeg via libdirac_* and Schroedinger libraries]
Anuradha Suraparaju
anuradha
Fri Apr 4 12:35:46 CEST 2008
On Fri, 2008-04-04 at 10:36 +0200, Diego Biurrun wrote:
> On Fri, Apr 04, 2008 at 02:39:25PM +1100, Anuradha Suraparaju wrote:
> > Sorry. Forgot to mention that the message is a patch in my earlier
> > email.
>
> Please don't top-post, you are going to become really unpopular in a
> short time if you do. I realize you are actually forwarding a message
> right now. I'm pointing this out just in case :)
OK.
>
> > I have attached a patch to FFmpeg to enable Dirac compression system
> > support via the libdirac_encoder/decoder libraries and Schroedinger
> > libraries. The attached README has instructions as to how to apply the
> > patch and enable Dirac support.
>
> OK, the first question that comes to my mind is: What is the difference
> between libdirac and libschrodinger and why would we need/want support
> for both?
libdirac is the C++ reference implementation of Dirac being implemented
by the BBC. It is not very optimised and hence not very fast. But the
compression performance is lots better than the of Schroedinger.
libschroedinger is an optimised, fast implementation of Dirac written in
C. Its compression perfomance lags behind Dirac and it might be a few
months before it catches up. So ideally we would want encoder support
using libdirac and decoder support using libschroedinger. But we haven't
tested libschroedinger on MS-Windows platforms yet. This is the reason I
included both libdirac and libschroedinger support.
>
> The second issue is that a native Dirac decoder and encoder was
> developed during Google Summer of Code 2007, but left unfinished.
> AFAIK the decoder is close to being finished. Maybe you can help with
> getting it merged into FFmpeg
I'll be glad to help.
> .
>
> On to the actual patch...
>
> Why is this a combined patch for both libs? Please split it into
> separate parts for libschrodinger and libdirac, this will be easier to
> review
Ok.
>
> You need to change the names of the files and variables. All external
> libraries sport a "lib" prefix in their name in FFmpeg to differentiate
> them from native implementations.
>
OK
> Please use "schrodinger" instead of "schro" as variable name and
> filename.
OK. Will change all occurences of schro to schroedinger.
>
> > --- ffmpegsvn_trunk/configure 2008-04-04 09:05:46.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/configure 2008-04-04 09:06:15.000000000 +1000
> > @@ -1640,6 +1658,8 @@
> > enabled mlib && require mediaLib mlib_types.h mlib_VectorSub_S16_U8_Mod -lmlib
> > +enabled dirac && add_extralibs `pkg-config --libs dirac` "-lstdc++" && extraincs="$extraincs `pkg-config --cflags dirac`"
> > +enabled schro && add_extralibs `pkg-config --libs schroedinger-1.0` && extraincs="$extraincs `pkg-config --cflags schroedinger-1.0`" && extraincs="$extraincs "
>
> Please keep the columns aligned.
OK
>
> > @@ -1852,6 +1872,8 @@
> > enabled libtheora && append pkg_requires "theora"
> > enabled libvorbis && append pkg_requires "vorbisenc"
> > +enabled dirac && append pkg_requires "dirac"
> > +enabled schro && append pkg_requires "schro"
>
> ditto
>
OK
> > @@ -2029,6 +2053,7 @@
> > fi
> > echo "LIB_INSTALL_EXTRA_CMD=${LIB_INSTALL_EXTRA_CMD}" >> config.mak
> > echo "EXTRALIBS=$extralibs" >> config.mak
> > +echo "EXTRAINCS=$extraincs" >> config.mak
>
> I'm suspicious of this change.
This is the only way I could think of of passing the Include directories
for Dirac and Schroedinger.
>
> > --- ffmpegsvn_trunk/libavcodec/avcodec.h 2008-04-01 09:14:03.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/avcodec.h 2008-04-01 09:15:43.000000000 +1000
> > @@ -182,6 +182,8 @@
> > CODEC_ID_8SVX_FIB,
> > CODEC_ID_ESCAPE124,
> > + CODEC_ID_SCHRO = 0xFFFE,
> > + CODEC_ID_DIRAC = 0xFFFF,
>
> Why the number?
While developing and testing the code, I didn't want the codec ids for
Dirac and Schroedinger to interfere with the ones being added. I'll
remove the numbers.
>
> > --- ffmpegsvn_trunk/libavcodec/dirac.c 1970-01-01 10:00:00.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/dirac.c 2008-04-04 11:28:12.000000000 +1000
> > @@ -0,0 +1,514 @@
> > +/*
> > + * Dirac support via Dirac libraries
> > + * Copyright (c) 2005 BBC, Andrew Kennedy <dirac at rd dot bbc dot co dot uk>
> > + * Copyright (c) 2006-2008 BBC, Anuradha Suraparaju <asuraparaju at gmail dot com >
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111-1307 USA
>
> Use a standard license header from other files.
OK.
>
> > --- ffmpegsvn_trunk/libavcodec/dirac_schro_parser.c 1970-01-01 10:00:00.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/dirac_schro_parser.c 2008-04-04 11:28:45.000000000 +1000
> > @@ -0,0 +1,228 @@
> > +/*
> > + * Common Dirac and Schroedinger parser
> > + * Copyright (c) 2008 BBC, Anuradha Suraparaju <asuraparaju at gmail dot com >
> > + *
> > + * This file is part of FFmpeg
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111-1307 USA
> > + */
>
> ditto
OK.
>
> > + if(p_videobuf_code[0]==0x42 && p_videobuf_code[1]==0x42 && p_videobuf_code[2]==0x43 && p_videobuf_code[3]==0x44)
>
> Please break these ugly long lines.
>
OK
> > --- ffmpegsvn_trunk/libavcodec/dirac_schro_parser.h 1970-01-01 10:00:00.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/dirac_schro_parser.h 2008-04-04 11:29:23.000000000 +1000
> > @@ -0,0 +1,73 @@
> > +/*
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111-1307 USA
> > + */
>
> license header once more
>
OK
> > +#ifndef FFMPEG_DIRAC_SCHRO_PARSER_H
> > +#define FFMPEG_DIRAC_SCHRO_PARSER_H
> > +
> > + [...]
> > +
> > +#endif
>
> Please comment the #endif.
>
OK
> > --- ffmpegsvn_trunk/libavcodec/Makefile 2008-04-03 09:32:52.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/Makefile 2008-04-03 09:33:23.000000000 +1000
> > @@ -4,6 +4,8 @@
> >
> > +CFLAGS += $(EXTRAINCS)
>
> As I said, this is suspicious.
>
> > --- ffmpegsvn_trunk/libavcodec/schroedinger.c 1970-01-01 10:00:00.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/schroedinger.c 2008-04-04 11:29:00.000000000 +1000
> > @@ -0,0 +1,700 @@
> > +/*
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111-1307 USA
> > + */
>
> see above
>
OK
> > --- ffmpegsvn_trunk/libavcodec/schro_parser.c 1970-01-01 10:00:00.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavcodec/schro_parser.c 2008-04-04 11:29:11.000000000 +1000
> > @@ -0,0 +1,29 @@
> > +/*
> > + * This file is part of FFmpeg
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111-1307 USA
> > + */
>
> .. and again ..
>
OK
> > +#include "dirac_schro_parser.h"
> > +
> > +AVCodecParser schro_parser = {
> > + { CODEC_ID_SCHRO },
> > + sizeof(FfmpegDiracSchroParseContext),
> > + dirac_schro_parse_open,
> > + dirac_schro_parse,
> > + dirac_schro_parse_close,
> > +};
>
> I have a feeling this could be merge into the other file.
>
OK.
> > --- ffmpegsvn_trunk/libavformat/allformats.c 2008-04-01 09:14:04.000000000 +1000
> > +++ ffmpegsvn_trunk_dirac_schro/libavformat/allformats.c 2008-04-03 14:34:57.000000000 +1000
> > @@ -68,6 +68,14 @@
> > + /*
> > + * Register SCHRO demuxer before Dirac so that Schro is used as the default
> > + * codec to decode Dirac bytestreams
> > + */
>
> Why?
>
As I said earlier, Schroedinger decoder is quicker than Dirac. Hence I'd
like to use Schroedinger decoder by default when both Schroedinger and
Dirac are enabled. Dirac is more of a backup decoder really on MS
Windows platforms when Schroedinger is not installed.
> Diego
Thanks for the feedback.
Regards,
Anuradha
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list