#1423: GRASS 6.4.1 won't compile against FFmpeg 0.8
-------------------------+--------------------------------------------------
Reporter: sharpie | Owner: grass-dev@…
Type: defect | Status: new
Priority: critical | Milestone:
Component: Default | Version: unspecified
Keywords: | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------
It appears some symbols used by the GRASS interface to FFmpeg were
depreciated in the latetest FFmpeg release:
{{{
/usr/bin/cc
-I/private/tmp/homebrew-grass-6.4.1-BgUk/grass-6.4.1/dist.i386-apple-
darwin10.8.0/include
-I/usr/local/Cellar/readline/6.2.1/include
-I/usr/local/Cellar/gettext/0.18.1.1/include -I/usr/local/include -O3
-march=core2 -msse4.1 -w -pipe -arch x86_64 -I/usr/local/include
-fno-common
-DPACKAGE=\""grasslibs"\" -I/usr/local/Cellar/gdal/1.8.1/include
-I/usr/local/Cellar/geos/3.3.0/include -DPACKAGE=\""grasslibs"\"
-I/System/Library/Frameworks/OpenGL.framework/Headers
-I/usr/local/include/libavcodec -I/usr/local/include/libavdevice
-I/usr/local/include/libavfilter -I/usr/local/include/libavformat
-I/usr/local/include/libavutil -I/usr/local/include/libpostproc
-I/usr/local/include/libswscale
-I/private/tmp/homebrew-grass-6.4.1-BgUk/grass-6.4.1/dist.i386-apple-
darwin10.8.0/include
-o OBJ.i386-apple-darwin10.8.0/gsd_img_mpeg.o -c gsd_img_mpeg.c
gsd_img_mpeg.c: In function ‘add_video_stream’:
gsd_img_mpeg.c:69: error: ‘CODEC_TYPE_VIDEO’ undeclared (first use in this
function)
gsd_img_mpeg.c:69: error: (Each undeclared identifier is reported only
once
gsd_img_mpeg.c:69: error: for each function it appears in.)
gsd_img_mpeg.c: In function ‘write_video_frame’:
gsd_img_mpeg.c:218: error: ‘PKT_FLAG_KEY’ undeclared (first use in this
function)
make[3]: *** [OBJ.i386-apple-darwin10.8.0/gsd_img_mpeg.o] Error 1
}}}
I've attached a minimal patch (attachment:ogsf.patch) to get OGSF to
compile with recent FFmpeg. Can someone test that it (i.e. generating
video from NVIZ) works?
Replying to [comment:2 glynn]:
> Replying to [comment:1 glynn]:
>
> I've attached a minimal patch (attachment:ogsf.patch) to get OGSF to
compile with recent FFmpeg. Can someone test that it (i.e. generating
video from NVIZ) works?
It compiles and produces output, still there are three issues:
* output video has problems with color. I'm not certain if it wasn't
present also before. Might be related to the next point:
* VBV buffer size not set, muxing may fail (see full message below)
{{{
Output #0, mpeg, to 'asd':
Stream #0:0: Video: mpeg1video (hq), yuv420p, 1021x766,
q=2-31, 400 kb/s, 90k tbn, 25 tbc
[mpeg @ 0x1b5f430] VBV buffer size not set, muxing may fail
[mpeg @ 0x1b5f430] buffer underflow i=0 bufi=0 size=62369
}}}
* AVMEDIA_TYPE_VIDEO was introduced only in 2010. There might still be
around older versions - needs a change in configure magic to refuse
compilation with older versions
I used this patch on Ubuntu 12.04 (32bit) too. But with this compiler
flags I can not compile it.
{{{
export CFLAGS="-ggdb -Wall -Werror-implicit-function-declaration"
}}}
Removing `-Werror-implicit-function-declaration` solved the problem but I
suppose it is not the solution. I also get some `-Wdeprecated-declaration`
warnings.
Replying to [comment:5 wenzeslaus]:
> Removing `-Werror-implicit-function-declaration` solved the problem but
I suppose it is not the solution. I also get some `-Wdeprecated-
declaration` warnings.
I don't have this problem with ffmpeg 0.10.3. There, av_rescale_q is
declared in libavutil/mathematics.h, which is included by <avutil.h>.
Also, your warnings show a couple of significant issues with gvld.c:
{{{
gvld.c: In function ‘gvld_isosurf’:
gvld.c:351:29: warning: operation on ‘*(pos + (unsigned int)((unsigned
int)i * 4u))’ may be undefined [-Wsequence-point]
gvld.c:351:29: warning: operation on ‘*(pos + (unsigned int)((unsigned
int)i * 4u))’ may be undefined [-Wsequence-point]
}}}
The code in question is:
{{{
if (check_color[i])
curcolor[i] =
(READ() & 0xff) | ((READ() & 0xff) << 8) |
((READ() & 0xff) << 16);
}}}
where the READ() macro is defined as:
{{{ #define READ() gvl_read_char(pos[i]++, gvl->isosurf[i]->data)
}}}
The problem is that, in an expression such as "A | B", if A and B have
side-effects, the C standard doesn't specify the order in which those
side-effects occur. I suspect that the code was written on the assumption
of left-to-right evaluation, in which case it should read e.g.:
{{{
if (check_color[i]) {
unsigned r = READ();
unsigned g = READ();
unsigned b = READ();
curcolor[i] =
(r & 0xff) | ((r & 0xff) << 8) |
((r & 0xff) << 16);
}}}
There's another bug on line 367:
{{{
gsd_set_material(1, 1, ksh[i], kem[i],
(int)curcolor);
}}}
This should probably have been:
{{{
gsd_set_material(1, 1, ksh[i], kem[i],
curcolor[i]);
}}}
Replying to [comment:2 glynn]:
> Replying to [comment:1 glynn]:
>
> I've attached a minimal patch (attachment:ogsf.patch) to get OGSF to
compile with recent FFmpeg. Can someone test that it (i.e. generating
video from NVIZ) works?
I have tried using FFMPEG 0.10 on Fedora 17/64bit:
Replying to [comment:9 neteler]:
> Glynn, should the patch be submitted?
It depends upon whether you're more concerned about the patch not working
with older versions of FFMPEG or the lack of a patch not working with
newer versions.
It wouldn't be hard to add some `#define`s, provided that we could figure
out the correct version check. E.g.
{{{ #if OLD_FFMPEG #define AVMEDIA_TYPE_VIDEO CODEC_TYPE_VIDEO #define AV_PKT_FLAG_KEY PKT_FLAG_KEY #define av_guess_format guess_format #define avformat_alloc_context av_alloc_format_context #endif
}}}
Replying to [comment:11 neteler]:
> Patch applied to GRASS 6.4 (r52978), 6.5 (r52979), and 7 (r52980).
>
> Keeping opened for potential conditionalizing for older FFMPEG versions.
see r53856, r53884, but as per comment:13, apparently more tuning still
needed.
there is still some trouble working around mathematics.h not being
included, but that's libav's bug not ours.
backported to relbr64 in r56737. reopen if further adjustments are needed,
and if you're updating to a newer version of the API, please leave some
conditionals to support the older ones..
Hamish
ps- using --with-ffmpeg and --with-tcltk-includes in trunk is not needed,
since ffmpeg was only used with tcl/tk NVIZ and tcl/tk code is removed in
trunk.
Replying to [comment:18 hamish]:
> ps- using --with-ffmpeg and --with-tcltk-includes in trunk is not
needed, since ffmpeg was only used with tcl/tk NVIZ and tcl/tk code is
removed in trunk.
Right! Removed --with-ffmpeg. The "--with-tcltk-includes" was not by
intention there.