[vlc-devel] [PATCH 2/3] opengl: Split texture uploading to a separate function

Rémi Denis-Courmont remi at remlab.net
Thu Mar 14 12:18:48 CET 2013


On Thu, 14 Mar 2013 12:25:42 +0200, Martin Storsjö <martin at martin.st>
wrote:
> This allows to reuse the repacking logic for subpictures as well,
> fixing display of subpictures on ES2, where GL_UNPACK_ROW_LENGTH
> isn't available.
> 
> ---
> 
> Felix, please test if it works better on iOS this time.
> 
> Tested on desktop linux with the glx vout, and on X on pandaboard
> with the gles2 vout.
> ---
>  modules/video_output/opengl.c |  108
>  +++++++++++++++++++++++------------------
>  1 file changed, 62 insertions(+), 46 deletions(-)
> 
> diff --git a/modules/video_output/opengl.c
b/modules/video_output/opengl.c
> index c1b8c05..ba03101 100644
> --- a/modules/video_output/opengl.c
> +++ b/modules/video_output/opengl.c
> @@ -692,6 +692,58 @@ error:
>      return NULL;
>  }
>  
> +static void Upload(int width, int height, int full_width, int
full_height,
> +                   int w_num, int w_den, int h_num, int h_den,
> +                   int pitch, int pixel_pitch,
> +                   int full_upload, const uint8_t *pixels,
> +                   int tex_target, int tex_format, int tex_type)
> +{
> +#ifndef GL_UNPACK_ROW_LENGTH
> +    if ( (pitch / pixel_pitch) != (unsigned int)
> +         ( width * w_num / w_den ) )
> +    {
> +        uint8_t *new_plane = malloc( full_width * full_height * w_num *
> h_num / (h_den * w_den ) * pixel_pitch );

Use xmalloc() or deal with errors.

> +        uint8_t *destination = new_plane;
> +        const uint8_t *source = pixels;
> +
> +        for( unsigned h = 0; h < (height * h_num / h_den) ; h++ )
> +        {
> +            memcpy( destination, source, (width * w_num / w_den) *
> pixel_pitch );
> +            source += pitch;
> +            destination += (full_width * w_num / w_den) * pixel_pitch;

This code desperately needs some temporary variables for repeated
computations. The compiler _should_ know that the values cannot change and
precompute them, but still...

> +        }
> +        if (full_upload)
> +            glTexImage2D( tex_target, 0, tex_format,
> +                          full_width  * w_num / w_den,
> +                          full_height * h_num / h_den,
> +                          0, tex_format, tex_type, new_plane );
> +        else
> +            glTexSubImage2D( tex_target, 0,
> +                             0, 0,
> +                             full_width  * w_num / w_den,
> +                             height * h_num / h_den,
> +                             tex_format, tex_type, new_plane );
> +        free( new_plane );
> +    } else {
> +#else
> +        glPixelStorei(GL_UNPACK_ROW_LENGTH, pitch / pixel_pitch);
> +#endif
> +        if (full_upload)
> +            glTexImage2D(tex_target, 0, tex_format,
> +                         full_width  * w_num / w_den,
> +                         full_height * h_num / h_den,
> +                         0, tex_format, tex_type, pixels);
> +        else
> +            glTexSubImage2D(tex_target, 0,
> +                            0, 0,
> +                            full_width  * w_num / w_den,
> +                            full_height * h_num / h_den,
> +                            tex_format, tex_type, pixels);
> +#ifndef GL_UNPACK_ROW_LENGTH
> +    }
> +#endif

Maybe just me, but I don't ifdef of a single bracket. It's ugly, tends to
break syntax hilighting and makes the code flow harder to follow.

> +}
> +
>  int vout_display_opengl_Prepare(vout_display_opengl_t *vgl,
>                                  picture_t *picture, subpicture_t
>                                  *subpicture)
>  {
> @@ -706,38 +758,10 @@ int
> vout_display_opengl_Prepare(vout_display_opengl_t *vgl,
>          }
>          glBindTexture(vgl->tex_target, vgl->texture[0][j]);
>  
> -#ifndef GL_UNPACK_ROW_LENGTH
> -        if ( (picture->p[j].i_pitch / picture->p[j].i_pixel_pitch) !=
> (unsigned int)
> -             ( picture->format.i_visible_width *
vgl->chroma->p[j].w.num
> / vgl->chroma->p[j].w.den ) )
> -        {
> -            uint8_t *new_plane = malloc(
picture->format.i_visible_width
> * vgl->fmt.i_visible_height * vgl->chroma->p[j].w.num *
> vgl->chroma->p[j].h.num / (vgl->chroma->p[j].h.den *
> vgl->chroma->p[j].w.den ) * picture->p[j].i_pixel_pitch );
> -            uint8_t *destination = new_plane;
> -            const uint8_t *source = picture->p[j].p_pixels;
> -
> -            for( unsigned height = 0; height <
(vgl->fmt.i_visible_height
> * vgl->chroma->p[j].h.num / vgl->chroma->p[j].h.den) ; height++ )
> -            {
> -                memcpy( destination, source,
> picture->format.i_visible_width * vgl->chroma->p[j].w.num /
> vgl->chroma->p[j].w.den * picture->p[j].i_pixel_pitch );
> -                source += picture->p[j].i_pitch;
> -                destination += picture->format.i_visible_width *
> vgl->chroma->p[j].w.num / vgl->chroma->p[j].w.den *
> picture->p[j].i_pixel_pitch;
> -            }
> -            glTexSubImage2D( vgl->tex_target, 0,
> -                             0, 0,
> -                             picture->format.i_visible_width *
> vgl->chroma->p[j].w.num / vgl->chroma->p[j].w.den,
> -                             vgl->fmt.i_height *
vgl->chroma->p[j].h.num
> / vgl->chroma->p[j].h.den,
> -                             vgl->tex_format, vgl->tex_type, new_plane
);
> -            free( new_plane );
> -        } else {
> -#else
> -            glPixelStorei(GL_UNPACK_ROW_LENGTH, picture->p[j].i_pitch /
> picture->p[j].i_pixel_pitch);
> -#endif
> -            glTexSubImage2D(vgl->tex_target, 0,
> -                            0, 0,
> -                            vgl->fmt.i_width  * vgl->chroma->p[j].w.num
/
> vgl->chroma->p[j].w.den,
> -                            vgl->fmt.i_height * vgl->chroma->p[j].h.num
/
> vgl->chroma->p[j].h.den,
> -                            vgl->tex_format, vgl->tex_type,
> picture->p[j].p_pixels);
> -#ifndef GL_UNPACK_ROW_LENGTH
> -        }
> -#endif
> +        Upload(picture->format.i_visible_width,
vgl->fmt.i_visible_height,
> +               vgl->fmt.i_width, vgl->fmt.i_height,
> +               vgl->chroma->p[j].w.num, vgl->chroma->p[j].w.den,
> vgl->chroma->p[j].h.num, vgl->chroma->p[j].h.den,
> +               picture->p[j].i_pitch, picture->p[j].i_pixel_pitch, 0,
> picture->p[j].p_pixels, vgl->tex_target, vgl->tex_format,
vgl->tex_type);
>      }
>  
>      int         last_count = vgl->region_count;
> @@ -794,13 +818,9 @@ int
vout_display_opengl_Prepare(vout_display_opengl_t
> *vgl,
>                                        r->fmt.i_x_offset *
>                                        r->p_picture->p->i_pixel_pitch;
>              if (glr->texture) {
>                  glBindTexture(GL_TEXTURE_2D, glr->texture);
> -                /* TODO set GL_UNPACK_ALIGNMENT */
> -#ifdef GL_UNPACK_ROW_LENGTH
> -                glPixelStorei(GL_UNPACK_ROW_LENGTH,
> r->p_picture->p->i_pitch / r->p_picture->p->i_pixel_pitch);
> -#endif
> -                glTexSubImage2D(GL_TEXTURE_2D, 0,
> -                                0, 0, glr->width, glr->height,
> -                                glr->format, glr->type,
> &r->p_picture->p->p_pixels[pixels_offset]);
> +                Upload(r->fmt.i_visible_width, r->fmt.i_visible_height,
> glr->width, glr->height, 1, 1, 1, 1,
> +                       r->p_picture->p->i_pitch,
> r->p_picture->p->i_pixel_pitch, 0,
> +                       &r->p_picture->p->p_pixels[pixels_offset],
> GL_TEXTURE_2D, glr->format, glr->type);
>              } else {
>                  glGenTextures(1, &glr->texture);
>                  glBindTexture(GL_TEXTURE_2D, glr->texture);
> @@ -812,13 +832,9 @@ int
vout_display_opengl_Prepare(vout_display_opengl_t
> *vgl,
>                  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER,
>                  GL_LINEAR);
>                  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S,
>                  GL_CLAMP_TO_EDGE);
>                  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T,
>                  GL_CLAMP_TO_EDGE);
> -                /* TODO set GL_UNPACK_ALIGNMENT */
> -#ifdef GL_UNPACK_ROW_LENGTH
> -                glPixelStorei(GL_UNPACK_ROW_LENGTH,
> r->p_picture->p->i_pitch / r->p_picture->p->i_pixel_pitch);
> -#endif
> -                glTexImage2D(GL_TEXTURE_2D, 0, glr->format,
> -                             glr->width, glr->height, 0, glr->format,
> glr->type,
> -                            
&r->p_picture->p->p_pixels[pixels_offset]);
> +                Upload(r->fmt.i_visible_width, r->fmt.i_visible_height,
> glr->width, glr->height, 1, 1, 1, 1,
> +                       r->p_picture->p->i_pitch,
> r->p_picture->p->i_pixel_pitch, 1,
> +                       &r->p_picture->p->p_pixels[pixels_offset],
> GL_TEXTURE_2D, glr->format, glr->type);
>              }
>          }
>      }

-- 
Rémi Denis-Courmont
Sent from my collocated server



More information about the vlc-devel mailing list