Re: [Gnash-commit] gnash ChangeLog backend/sound_handler_sdl.cpp b...

[strk]

On Fri, Nov 17, 2006 at 02:52:19PM +0000, Tomas Groth wrote:

>               for(uint32_t i=0; i < 
> m_sound_data[handle_id]->m_active_sounds.size(); i++) {
> -                     m_sound_data[handle_id]->m_active_sounds[i]->data = 
> m_sound_data[handle_id]->data;
> +                     
> m_sound_data[handle_id]->m_active_sounds[i]->set_data(m_sound_data[handle_id]->data);
>                       m_sound_data[handle_id]->m_active_sounds[i]->data_size 
> = m_sound_data[handle_id]->data_size;
>                       m_sound_data[handle_id]->m_active_sounds[i]->position = 
> m_sound_data[handle_id]->data_size;
> -                     m_sound_data[handle_id]->m_active_sounds[i]->raw_data = 
> m_sound_data[handle_id]->data;
> +                     
> m_sound_data[handle_id]->m_active_sounds[i]->set_raw_data(m_sound_data[handle_id]->data);

Thomas, rather then keep using operator[] and operator-> everywhere,
it would be more readable to use intermediate variables. The compiler
would likely optimize them away anyway. For example:


        sound_data data = m_sound_data[handle_id];
        active_sounds asounds = data->m_active_sounds;

        for(uint32_t i=0; i < asounds.size(); ++i)
        {
                active_sound sound = asounds[i];
                sound->data = ..
                sound->set_data(...)    
        }

> +// Pointer handling and checking functions
> +uint8_t* active_sound::get_raw_data_ptr(unsigned long int pos) {
> +     assert(raw_data_size > pos);
> +     return raw_data + pos;
> +}
> +
> +uint8_t* active_sound::get_data_ptr(unsigned long int pos) {
> +     assert(data_size > pos);
> +     return data + pos;
> +}
> +
> +void active_sound::set_raw_data(uint8_t* iraw_data) {
> +     raw_data = iraw_data;
> +}
> +
> +void active_sound::set_data(uint8_t* idata) {
> +     data = idata;
> +}
> +
> +void active_sound::delete_raw_data() {
> +     delete [] raw_data;
> +}

Nice assertion checking!  :)

>  // AS-volume adjustment
>  void adjust_volume(int16_t* data, int size, int volume)
>  {
> @@ -657,8 +681,8 @@
>       } else {
>               startpos = sound->raw_position;
>       }
> -     assert(sound->raw_data_size > startpos);
> -     int16_t* data = (int16_t*) (sound->raw_data + startpos);
> +
> +     int16_t* data = (int16_t*) (sound->get_raw_data_ptr(startpos));

C-style casts are dangerous, are you sure you need it ?

> +do_mixing(Uint8* stream, active_sound* sound, Uint8* data, unsigned int 
> mix_length, unsigned int volume) {
> +     // If the volume needs adjustments we call a function to do that
> +     if (volume != 100) {
> +             adjust_volume((int16_t*)(data), mix_length, volume);

C-style cast again

--strk;