libvisual-devel

[Libvisual-devel] plugins

[Duilio J. P. <dp...@fc...>]

I have submitted many things to plugins package.

Before to ckeckout, run 'make distclean', then
checkout, then run autogen.sh.


Duilio.

Re: [Libvisual-devel] plugins

[Dennis S. <sy...@yo...>]

On Sat, 2004-06-26 at 16:40 -0300, Duilio Javier Protti wrote:
> I have submitted many things to plugins package.
> 
> Before to ckeckout, run 'make distclean', then
> checkout, then run autogen.sh.
> 
> 
> Duilio.


Cool. thanks a lot! :)

Btw now you're getting more indepth with the system, how
do you like the overall design ? ofcourse there are some
rough edges here and there, both API, design and
implementation wise...

[Libvisual-devel] design

[Duilio J. P. <dp...@fc...>]

> Btw now you're getting more indepth with the system, how
> do you like the overall design ? ofcourse there are some
> rough edges here and there, both API, design and
> implementation wise...

I think the module design is perfect, however I have
not seen exhaustively the interface offered by every
module.

I think next step must be enforce all preconditions to
catch bugs:

int visual_some_thing (VisSome *vis)
{
  if (vis == NULL)
    return -1;
}

must be changed for:

int visual_some_thing (VisSome *vis)
{
  assert (vis != NULL);
  ...
}

This way we can catch them early, before the bug
propagate to obscure places. This is on the devel
line and just for a while, to clean up things and
to detect where the problems are, before to
change the VisBin module or create the new param
module.
On the final release we will use a nice macro 
_lv_return_if_null(), as you suggest on the TODO
list, that will show a critical warning but will
not abort.


Duilio.

[Libvisual-devel] xmms plugin

[Dennis S. <sy...@yo...>]

Duilio,

visual_video_depth_enum_from_value


In the xmms plugin within the visual_initialize
function you use:


        switch (lv_depth) {
        case 8:
            depth = VISUAL_VIDEO_DEPTH_8BIT;
        case 16:
            depth = VISUAL_VIDEO_DEPTH_16BIT;
        case 24:
            depth = VISUAL_VIDEO_DEPTH_24BIT;
        case 32:
            depth = VISUAL_VIDEO_DEPTH_32BIT;
            break;
        default:
            depth = lv_depth = VISUAL_VIDEO_DEPTH_24BIT;
            break;
        }

I'd like to point out that libvisual has a function:
visual_video_depth_enum_from_value that does this.


Dennis

Re: [Libvisual-devel] design

[Dennis S. <sy...@yo...>]

On Sun, 2004-06-27 at 12:42 -0300, Duilio Javier Protti wrote:
> I think next step must be enforce all preconditions to
> catch bugs:
> 
> int visual_some_thing (VisSome *vis)
> {
>   if (vis == NULL)
>     return -1;
> }
> 
> must be changed for:
> 
> int visual_some_thing (VisSome *vis)
> {
>   assert (vis != NULL);
>   ...
> }

I'm not sure about this, I do agree tho that we should
do extensive error checking using visual_log, I think
this should be sufficient and this way we don't have to
change the code. Maybe it's best tho to just use 
return_if_null macro in which we can do an assert
temporarly.

> This way we can catch them early, before the bug
> propagate to obscure places. This is on the devel
> line and just for a while, to clean up things and
> to detect where the problems are, before to
> change the VisBin module or create the new param
> module.
> On the final release we will use a nice macro 
> _lv_return_if_null(), as you suggest on the TODO
> list, that will show a critical warning but will
> not abort.
> 
> 
> Duilio.

Re: [Libvisual-devel] design

[Duilio J. P. <dp...@fc...>]

> I'm not sure about this, I do agree tho that we should
> do extensive error checking using visual_log, I think
> this should be sufficient and this way we don't have to
> change the code. Maybe it's best tho to just use 
> return_if_null macro in which we can do an assert
> temporarly.

Ok, that's true, that way we need less code rewriting.
I will see how that macro must be.


Duilio.