[GRASS5] [bug #4246] (grass) NVIZ: segfault in kanimation panel

GRASS GIS GRASS GIS developer
1

re. segfault in nviz if you use a d.nviz generated script:

G61> nviz script=somefile.nvsrc

The segfaults happens when it tries to add a keyframe:

visualization/nviz/src/anim_support.c
line ~410 in Nadd_key_cmd()

   G_free (listels);

if I comment that line out, the script runs, nviz loads, and all is good.

printf(%p) shows listels at 0xbfffee6c

I notice in the compiler warnings:
anim_support.c:367: warning: passing arg 4 of `Tcl_SplitList' from
incompatible pointer type

which is this:
    if (Tcl_SplitList(interp, argv[2], &numels, &listels) != TCL_OK)

is that call promoting listels to something else?

Is there a better way of debugging nviz than just peppering puts and printf's?
gdb is useless for it.

thanks,
Hamish

-------------------------------------------- Managed by Request Tracker

2

Hamish,

I took a quick look at this and I cannot duplicate the error. I do get
the compiler warning for all of the Tcl_SplitList in src. According to
TCL recent documentation arg 4 in Tcl_SplitList should actually be
"const char **". If I change
char **listels;
in anim_support.c (line ~933)
to const char **listels;
I do not get the compiler warning.

Also, according to the Tcl_SplitList documentation, the **listels
argument should be freed. They recommend using
Tcl_Free((char *) argv);

I am not sure why the free call would be creating a problem.

Hope this helps.

--
Bob

On Sat, 2006-04-08 at 13:10 +0200, Harmish Bowman via RT wrote:

re. segfault in nviz if you use a d.nviz generated script:

G61> nviz script=somefile.nvsrc

The segfaults happens when it tries to add a keyframe:

visualization/nviz/src/anim_support.c
line ~410 in Nadd_key_cmd()

   G_free (listels);

if I comment that line out, the script runs, nviz loads, and all is good.

printf(%p) shows listels at 0xbfffee6c

I notice in the compiler warnings:
anim_support.c:367: warning: passing arg 4 of `Tcl_SplitList' from
incompatible pointer type

which is this:
    if (Tcl_SplitList(interp, argv[2], &numels, &listels) != TCL_OK)

is that call promoting listels to something else?

Is there a better way of debugging nviz than just peppering puts and printf's?
gdb is useless for it.

thanks,
Hamish

-------------------------------------------- Managed by Request Tracker

_______________________________________________
grass5 mailing list
grass5@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass5

3

I took a quick look at this and I cannot duplicate the error.

I am using Debian with TclTk 8.4, which is a new thing. See:
  http://article.gmane.org/gmane.comp.gis.grass.devel/11177
  http://grass.itc.it/pipermail/grass-commit/2006-March/021104.html

Maybe a similar issue?

(I haven't recompiled with tcltk 8.3 yet to check if this is the case,
obviously it would be a good idea to do so. It works fine with a CVS
version from February (compiled with tcltk 8.3), so I would guess this
is a pretty strong lead)

I do get the compiler warning for all of the Tcl_SplitList in src.
According to TCL recent documentation arg 4 in Tcl_SplitList should
actually be "const char **". If I change
char **listels;
in anim_support.c (line ~933)
to const char **listels;
I do not get the compiler warning.

If I add "const", the warning goes away for me too. But it still
segfaults on Load anim file.

I also notice these warnings:

<command line>:5:1: warning: "__STDC__" redefined
In file included from anim_support.c:11:
interface.h:363: warning: `struct Map_info' declared inside parameter list
interface.h:363: warning: its scope is only this definition or declaration, which is probably not what you want

and eight "warning: passing arg 1 of `...' as `float' rather than `double' due to prototype"

Also, according to the Tcl_SplitList documentation, the **listels
argument should be freed. They recommend using
Tcl_Free((char *) argv);

I am not sure why the free call would be creating a problem.

see above links ??

I've made a number of small cleanups to the kanim code in the last week
to make things a little easier on the user (also d.nviz, nviz2.2_script).
Hopefully it will just be smoother and the changes won't be noticed.
The Keyframe Animator help page is extended including an example and
should be easier to follow.

Also I added an #ifdef to the MPEG code to make it easy to switch to
using the XVID codec. (compile with gcc -D"USE_XVID") This should really
be an enviro var or GIS var setting, but for now it's a quick proof of
concept hack.

Result of this is that making movies is somewhat easier now. Great
synergy with r.in.wms(x2) or r.in.onearth scripts to fetch SRTM and
LANDSAT. Process LANDSAT image with Markus's i.landsat.rgb & even
better.

Q: If I make a track with d.nviz the camera looks straight down, not
forwards along the track. Is this normal/expected?

cheers,
Hamish

4

On Wed, 2006-04-12 at 15:07 +1200, Hamish wrote:

> I took a quick look at this and I cannot duplicate the error.

I am using Debian with TclTk 8.4, which is a new thing. See:
  http://article.gmane.org/gmane.comp.gis.grass.devel/11177
  http://grass.itc.it/pipermail/grass-commit/2006-March/021104.html

Maybe a similar issue?

(I haven't recompiled with tcltk 8.3 yet to check if this is the case,
obviously it would be a good idea to do so. It works fine with a CVS
version from February (compiled with tcltk 8.3), so I would guess this
is a pretty strong lead)

> I do get the compiler warning for all of the Tcl_SplitList in src.
> According to TCL recent documentation arg 4 in Tcl_SplitList should
> actually be "const char **". If I change
> char **listels;
> in anim_support.c (line ~933)
> to const char **listels;
> I do not get the compiler warning.

If I add "const", the warning goes away for me too. But it still
segfaults on Load anim file.

I also notice these warnings:

<command line>:5:1: warning: "__STDC__" redefined
In file included from anim_support.c:11:
interface.h:363: warning: `struct Map_info' declared inside parameter list
interface.h:363: warning: its scope is only this definition or declaration, which is probably not what you want

and eight "warning: passing arg 1 of `...' as `float' rather than `double' due to prototype"

I also get these warnings?

> Also, according to the Tcl_SplitList documentation, the **listels
> argument should be freed. They recommend using
> Tcl_Free((char *) argv);
>
> I am not sure why the free call would be creating a problem.

see above links ??

Taking a look at some of the other Tcl_SplitList calls (for example in
draw.c) the char is freed with G_free ((char *) ...). I would expect
these to also segfault if it was a problem with G-free. Try simply
changing
G_free (listels);
to
G_free ((char *) listels);

I've made a number of small cleanups to the kanim code in the last week
to make things a little easier on the user (also d.nviz, nviz2.2_script).
Hopefully it will just be smoother and the changes won't be noticed.
The Keyframe Animator help page is extended including an example and
should be easier to follow.

These all sound great. The extended help is a good idea as the Keyframe
Animator can be quite difficult to learn.

Also I added an #ifdef to the MPEG code to make it easy to switch to
using the XVID codec. (compile with gcc -D"USE_XVID") This should really
be an enviro var or GIS var setting, but for now it's a quick proof of
concept hack.

Instead of a compile flag could we try something like ...
codec = avcodec_find_encoder(CODEC_ID_XVID);
if (!codec)
{
  codec = avcodec_find_encoder(CODEC_ID_MPEG1VIDEO);
  if (!codec) {
    fprintf(stderr, "codec not found\n");
    return(-1);
  }
}
This way it tries to load the better quality codec, and if it fails it
will load the old standard mpeg1. I have not tested it to see if it
works.

Result of this is that making movies is somewhat easier now. Great
synergy with r.in.wms(x2) or r.in.onearth scripts to fetch SRTM and
LANDSAT. Process LANDSAT image with Markus's i.landsat.rgb & even
better.

Q: If I make a track with d.nviz the camera looks straight down, not
forwards along the track. Is this normal/expected?

If the camera is looking down then you need to increase the dist
parameter. The combination of the ht and dist parameters are what
determine the camera look angle.

cheers,
Hamish

Hope this helps.

--
Bob

5

> I am using Debian with TclTk 8.4, which is a new thing. See:
> http://article.gmane.org/gmane.comp.gis.grass.devel/11177
> http://grass.itc.it/pipermail/grass-commit/2006-March/021104.html
>
> Maybe a similar issue?

..

> > I do get the compiler warning for all of the Tcl_SplitList in src.
> > According to TCL recent documentation arg 4 in Tcl_SplitList
> > should actually be "const char **". If I change
> > char **listels;
> > in anim_support.c (line ~933)
> > to const char **listels;
> > I do not get the compiler warning.
>
> If I add "const", the warning goes away for me too. But it still
> segfaults on Load anim file.
>
> I also notice these warnings:
>
> <command line>:5:1: warning: "__STDC__" redefined
> In file included from anim_support.c:11:
> interface.h:363: warning: `struct Map_info' declared inside
> parameter list interface.h:363: warning: its scope is only this
> definition or declaration, which is probably not what you want

> > Also, according to the Tcl_SplitList documentation, the **listels
> > argument should be freed. They recommend using
> > Tcl_Free((char *) argv);
> >
> > I am not sure why the free call would be creating a problem.
>
> see above links ??

Taking a look at some of the other Tcl_SplitList calls (for example in
draw.c) the char is freed with G_free ((char *) ...). I would expect
these to also segfault if it was a problem with G-free. Try simply
changing
G_free (listels);
to
G_free ((char *) listels);

Nope, still segfaults.

> Also I added an #ifdef to the MPEG code to make it easy to switch to
> using the XVID codec. (compile with gcc -D"USE_XVID") This should
> really be an enviro var or GIS var setting, but for now it's a quick
> proof of concept hack.

Instead of a compile flag could we try something like ...
codec = avcodec_find_encoder(CODEC_ID_XVID);
if (!codec)
{
  codec = avcodec_find_encoder(CODEC_ID_MPEG1VIDEO);
  if (!codec) {
    fprintf(stderr, "codec not found\n");
    return(-1);
  }
}
This way it tries to load the better quality codec, and if it fails it
will load the old standard mpeg1. I have not tested it to see if it
works.

Maybe someone wants something more than XVID or MPEG-1?
(shrug- anybody?)

XVID requires the canvas to be some multiple of 16x16, and fails to
load (without telling why) or produces artifacts in the output if not.
So as a default it may be tricky for newcomers.. But most/all other
encoders have the same problem. (including MPEG-1??)

> Q: If I make a track with d.nviz the camera looks straight down, not
> forwards along the track. Is this normal/expected?

If the camera is looking down then you need to increase the dist
parameter. The combination of the ht and dist parameters are what
determine the camera look angle.

Oh, ok. I was keeping dist= small as I was getting out-of-region
messages and skipped points if I went within that distance to the edge
of the region. Any thoughts on extending the module to work with a
vector line? Maybe needs a cat= option tp pick specific line and a -r
reverse flag to follow it the other direction. I'm thinking flying above
a road or weaking the flight path in v.digit.

Hamish