On Sun, Sep 23, 2012 at 07:00:14PM +0200, Jordi Ortiz wrote:
>
> --- a/libavcodec/dwt.c
> +++ b/libavcodec/dwt.c
> @@ -21,6 +21,9 @@
> #include "libavutil/attributes.h"
> #include "libavutil/common.h"
> #include "dsputil.h"
> +#if CONFIG_SNOW_DECODER
> +#include "snow_dwt.h"
> +#endif
Why the #ifdefs? The header does not contain tables or anything
that might break if snow is disabled. Only use #ifdefs if you
have a good reason, not "just in case".
> @@ -870,10 +833,12 @@ void ff_dsputil_init_dwt(DSPContext *c)
>
> void ff_dwt_init(DWTContext *c)
> {
> +#if CONFIG_SNOW_DECODER
> c->vertical_compose97i = ff_snow_vertical_compose97i;
> c->horizontal_compose97i = ff_snow_horizontal_compose97i;
> c->inner_add_yblock = ff_snow_inner_add_yblock;
>
> if (HAVE_MMX)
> ff_dwt_init_x86(c);
> +#endif
> }
If all this does is initialize snow-specific functions, then it should
probably be in snow_dsp.c and have a better name.
> --- /dev/null
> +++ b/libavcodec/snow_dwt.c
> @@ -0,0 +1,44 @@
> +#include "snow_dwt.h"
> +
You are missing the #include for IDWT_ELEM.
And where is the license boilerplate?
> --- /dev/null
> +++ b/libavcodec/snow_dwt.h
> @@ -0,0 +1,10 @@
> +#include "dwt.h"
Where is the license boilerplate? And where are the multiple inclusion
guards?
Apparently you were not aware that they are necessary. Would it have
helped to find this information in the developer documentation?
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
