[vlc-devel] [vlc-commits] commit: vmem: By default keep the original image aspect ratio. (Pierre d' Herbemont )

[Pierre d'Herbemont]

On Sun, Oct 31, 2010 at 12:32 PM, Laurent Aimar <fenrir at elivagar.org> wrote:
> Hi
>
>> @@ -266,6 +294,7 @@ static int Control(vout_display_t *vd, int query, va_list args)
>>              return VLC_EGENERIC;
>>          if (cfg->is_fullscreen)
>>              return VLC_EGENERIC;
>> +
> Cosmetics changes...
>
>> -    for (int i = 0; i < picture->i_planes; i++)
>> -        picture->p[i].p_pixels = planes[i];
>> +    const int y = sys->place.y;
>> +    const int x = sys->place.x;
>> +    const int pitch = sys->vmem_pitch;
>> +    const int bbp = pitch / sys->vmem_width;
>  This is wrong, the pitch is not a multiple of the width.

No. "vmem pitch" doesn't include any extra trailer bytes. In vmem
width * bbp = pitch. Else this is broken. That's also a good reason
why vmem API sucks.

> Use picture->p[i].i_pixel_pitch instead.

Nice shortcut. No idea this was even existing. Why don't we have bpp directly?

>> +    for (int i = 0; i < picture->i_planes; i++) {
>> +        uint8_t *p = planes[i];
>  Why keeping the intermediate planes[] buffer? You should be able to
> simplify by using picsys->original_planes, no?

I have absolutely no idea.

>> +        picsys->original_planes[i] = p;
>> +        picture->p[i].p_pixels = p + x * bbp + y * pitch;
>> +        picture->p[i].i_lines = sys->place.height;
>> +    }
>  Also, it seems that you suppose that the vmem users initializes the picture
> to a sensible color for the border. You should at least document it.

Good idea.

>>      return VLC_SUCCESS;
>>  }
>> @@ -300,7 +338,7 @@ static void Unlock(picture_t *picture)
>>      void *planes[PICTURE_PLANE_MAX];
>>
>>      for (int i = 0; i < picture->i_planes; i++)
>> -        planes[i] = picture->p[i].p_pixels;
>> +        planes[i] = picsys->original_planes[i];
>>
>>      if (sys->unlock != NULL)
>>          sys->unlock(sys->opaque, picsys->id, planes);
>  Same there (planes[]).

Thanks!

Pierre.