[vlc-devel] [PATCH 10/11] Add KVA video output module for OS/2

[KO Myung-Hun]

Rémi Denis-Courmont wrote:
> Le dimanche 6 novembre 2011 11:58:29 KO Myung-Hun, vous avez écrit :
>> +/*************************************************************************
>> **** + * Manage: handle Sys events
>> +
>> **************************************************************************
>> *** + * This function should be called regularly by video output thread. It
>> returns + * a non null value if an error occurred.
>> +
>> **************************************************************************
>> ***/ +static void Manage( vout_display_t *vd )
>> +{
>> +    vout_display_sys_t * sys = vd->sys;
>> +
>> +    /* Let a window procedure manage instead because if resizing a frame
>> window +     * here, WM_SIZE is not sent to its child window.
>> +     * Maybe, is this due to the different threads ? */
>> +    WinPostMsg( sys->client, WM_VLC_MANAGE, 0, 0 );
>> +}
> 
> I don't understand why you need this if you have a dedicated thread anyway. 
> You should leave pf_manage to NULL.
> 

To check the changes of a parent window. Without this, an embedded
window cannot determine its size corresponding to its parent window.

>> +
>> +/*************************************************************************
>> **** + * Control: control facility for the vout
>> +
>> **************************************************************************
>> ***/ +static int Control( vout_display_t *vd, int query, va_list args ) +{
>> +    vout_display_sys_t *sys = vd->sys;
>> +
>> +    switch (query)
>> +    {
>> +    case VOUT_DISPLAY_HIDE_MOUSE:
>> +    {
>> +        POINTL ptl;
>> +
>> +        WinQueryPointerPos( HWND_DESKTOP, &ptl );
>> +        if( WinWindowFromPoint( HWND_DESKTOP, &ptl, TRUE ) == sys->client
>> ) +        {
>> +            WinShowPointer( HWND_DESKTOP, FALSE );
>> +            sys->is_mouse_hidden = true;
>> +        }
>> +
>> +        return VLC_SUCCESS;
>> +    }
>> +
>> +    case VOUT_DISPLAY_CHANGE_FULLSCREEN:
>> +    {
>> +        vout_display_cfg_t cfg = *va_arg(args, const vout_display_cfg_t
>> *); +
>> +        if( sys->parent_window )
>> +            vout_window_SetFullScreen(sys->parent_window,
>> cfg.is_fullscreen); +        else
>> +            sys->i_changes |= KVA_FULLSCREEN_CHANGE;
> 
> The way you use sys->i_changes does not seem thread-safe.
> 

Ok, I'll use a mutex for this.

>> +static void ImageDisplay( image_t *p_image )
>> +{
>> +    PVOID p_kva_buffer;
>> +    ULONG i_kva_bpl;
>> +
>> +    if (!kvaLockBuffer(&p_kva_buffer, &i_kva_bpl))
>> +    {
>> +        uint8_t *dst[ 3 ] = { NULL };
>> +        int      dstStride[ 3 ] = { 0 };
>> +        int      i_bpl, i_height;
>> +
>> +        /* Get packed or Y */
>> +        dst[0]       = p_kva_buffer;
>> +        dstStride[0] = i_kva_bpl;
>> +
>> +        i_bpl    = p_image->w * p_image->i_bpp;
>> +        i_height = p_image->h;
>> +
>> +        /* Copy packed or Y */
>> +        ImageCopy(dst[0], p_image->data[0], i_bpl, i_height,
>> +                  dstStride[0], p_image->linesize[0]);
>> +
>> +        /* YV12 or YVU9 ? */
>> +        if( p_image->i_chroma_shift )
>> +        {
>> +            /* Get V */
>> +            dst[ 1 ]       = dst[ 0 ] + p_image->h * dstStride[ 0 ];
>> +            dstStride[ 1 ] = dstStride[ 0 ] >> p_image->i_chroma_shift;
>> +
>> +            /* Get U */
>> +            dst[ 2 ]       = dst[ 1 ] +
>> +                           ( p_image->h >> p_image->i_chroma_shift ) *
>> dstStride[ 1 ]; +            dstStride[ 2 ] = dstStride[ 1 ];
>> +
>> +            i_bpl >>= p_image->i_chroma_shift;
>> +            i_height >>= p_image->i_chroma_shift;
>> +
>> +            /* Copy V */
>> +            ImageCopy(dst[1], p_image->data[1], i_bpl, i_height,
>> +                      dstStride[1], p_image->linesize[1]);
>> +
>> +            /* Copy U */
>> +            ImageCopy(dst[2], p_image->data[2], i_bpl, i_height,
>> +                      dstStride[2], p_image->linesize[2]);
>> +        }
> 
> Ideally, you should copy the image from Render rather than Display.
> 

What is Render ?

> In any case, you are going to copy the image to video buffers manually. So I 
> don't see why you limit the picture pool to one single image. Nothing seems to 
> prevent you from allocating more than one VLC picture. Laurent would know 
> better, but I think a single picture is very suboptimal for decoding 
> performance.
> 

Would you mind providing examples and docs to refer to ?

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.0.14
Under OS/2 Warp 4 for Korean with FixPak #15
On AMD ThunderBird 1GHz with 512 MB RAM

Korean OS/2 User Community : http://www.ecomstation.co.kr