Bug#789250: bs1770gain: Fail to build on archs with strict alignment requirements

[Peter Belkner]

On 24.06.2015 12:41, Petter Reinholdtsen wrote:
> [Peter Belkner]
>> BS1770GAIN doesn't make any assumption on buffer alignment itself
>> (i.e.  it relies on FFmpeg), and so myself. Are there any hints that
>> FFmpeg is broken on the systems in question?
> I do not know.  But I am more worried about other alignment problems
> being hidden if the warning is disabled.
>
>> You may use SoX to generate test files.
> Thank you.  I'll try to figure out how to use sox for this.  In the mean
> time, I tested on mips if I could find a way to tell the compiler that
> the pointer is expected to have the correct alignment, and this patch
> solve the build problem related to AVFrame.data:
>
> --- bs1770gain-0.4.3.orig/libffsox-2/ffsox_frame_convert.c
> +++ bs1770gain-0.4.3/libffsox-2/ffsox_frame_convert.c
> @@ -140,10 +140,10 @@ static int convert_##r##i##_##w##i(conve
>     R *rp; \
>     W *wp,*mp; \
>    \
> -  rp=(R *)p->fr->frame->data[0]; \
> +  rp=(R *)(void*)p->fr->frame->data[0]; \
>     rp+=channels*p->fr->nb_samples.frame; \
>    \
> -  wp=(W *)p->fw->frame->data[0]; \
> +  wp=(W *)(void*)p->fw->frame->data[0]; \
>     wp+=channels*p->fw->nb_samples.frame; \
>     mp=wp+channels*p->nb_samples; \
>    \
> @@ -281,11 +281,11 @@ static int convert_##r##p##_##w##i(conve
>     W *wp,*mp; \
>    \
>     for (ch=0;ch<channels;++ch) { \
> -    rp[ch]=(R *)p->fr->frame->data[ch]; \
> +    rp[ch]=(R *)(void*)p->fr->frame->data[ch]; \
>       rp[ch]+=p->fr->nb_samples.frame; \
>     } \
>    \
> -  wp=(W *)p->fw->frame->data[0]; \
> +  wp=(W *)(void*)p->fw->frame->data[0]; \
>     wp+=channels*p->fw->nb_samples.frame; \
>     mp=wp+channels*p->nb_samples; \
>    \
> --- bs1770gain-0.4.3.orig/libffsox-2/ffsox_frame_convert_sox.c
> +++ bs1770gain-0.4.3/libffsox-2/ffsox_frame_convert_sox.c
> @@ -98,10 +98,10 @@ static void convert_##sfx##i(convert_t *
>    \
>     (void)ch; \
>    \
> -  rp=(T *)p->fr->frame->data[0]; \
> +  rp=(T *)(void*)p->fr->frame->data[0]; \
>     rp+=channels*p->fr->nb_samples.frame; \
>    \
> -  wp=(sox_sample_t *)p->fw->frame->data[0]; \
> +  wp=(sox_sample_t *)(void*)p->fw->frame->data[0]; \
>     wp+=channels*p->fw->nb_samples.frame; \
>     mp=wp+channels*p->nb_samples; \
>    \
> @@ -184,11 +184,11 @@ static void convert_##sfx##p(convert_t *
>     SOX_SAMPLE_LOCALS; \
>    \
>     for (ch=0;ch<channels;++ch) { \
> -    rp[ch]=(T *)p->fr->frame->data[ch]; \
> +    rp[ch]=(T *)(void*)p->fr->frame->data[ch]; \
>       rp[ch]+=p->fr->nb_samples.frame; \
>     } \
>    \
> -  wp=(sox_sample_t *)p->fw->frame->data[0]; \
> +  wp=(sox_sample_t *)(void*)p->fw->frame->data[0]; \
>     wp+=channels*p->fw->nb_samples.frame; \
>     mp=wp+channels*p->nb_samples; \
>    \
>
> I like this approach better, as it do not disable strict alignment
> checking for the entire code, but only for the selected parts where it
> is believed to be OK to disable it.  So I plan to keep the
> -Wstrict-align warning in place for the Debian package for now.

OK.

>
> Changing the code like that exposes another build error, the cast to
> (pull_cb_t) from char * done in this function:
>
>    static int getopts(sox_effect_t *e, int argc, char *argv[])
>    {
>      priv_t *priv=e->priv;
>
>      priv->cb=1<argc?(pull_cb_t)argv[1]:NULL;
>      priv->data=2<argc?(void *)argv[2]:NULL;
>
>      return SOX_SUCCESS;
>    }
>
> I do not quite understand why a char * is used to store a function
> pointer, and am thus reluctant to add (void*) until it is analyzed a bit
> better.

It's a similar problem with the SoX API. The SoX API requires one to 
pass arguments to an effect via the function "int 
sox_effect_options(sox_effect_t *effp, int argc, char * const argv[])" 
(http://sox.sourceforge.net/libsox.html), i.e. as "char * const argv[]". 
In the effect's "getopts()" method, the effect has to cast back the real 
type of the argument.

It's C after all.

If you like you can avoid the warning also by an additional "(void *)" cast.