[vlc-devel] [Patches]: Video Decoder Acceleration (VDA) avcodec module for Mac OS X

[Sebastien Zwickert]

Hi,

Thanks for reviewing. I attached an updated patch.

On Nov 2, 2011, at 8:51 PM, Laurent Aimar wrote:
> On Tue, Nov 01, 2011 at 04:07:58PM +0100, Sebastien Zwickert wrote:
>> […]
>> +
>> +static int Setup( vlc_va_t *p_external, void **pp_hw_ctx, vlc_fourcc_t *pi_chroma,
>> +                  int i_width, int i_height )
>> +{
>> +
>> +    vlc_va_vda_t *p_va = vlc_va_vda_Get(p_external);
>> +
>> +    if ( p_va->hw_ctx.decoder != NULL )
>> +    {
>> +        *pp_hw_ctx = &p_va->hw_ctx;
>> +        *pi_chroma = VLC_CODEC_UYVY;
>> +        return VLC_SUCCESS;
>> +    }
> This won't work. You must reinitialize when Setup() is called as some
> parameters may have change (like the width/height).

Fixed. The extra data may have changed too ?

> 
>> +    if ( pi_chroma == VLC_FOURCC( 'a', 'v', 'c', '1') )
> pi_chroma is a pointer…

I removed this useless check.

> 
>> +    {
>> +        msg_Warn( p_va->p_log, "Not avc." );
>> +        return VLC_EGENERIC;
>> +    }
>> +
>> +    *pp_hw_ctx = NULL;
>> +    *pi_chroma = VLC_CODEC_UYVY;
> Can't YV12/NV12/I420 be used?

I implemented Yv12 support. The conversion uyvy to yuv is done by VDA (VideoToolbox).
Yuv bi planar is only supported since 10.7 sdk.

>> [...]
>> +static int Extract( vlc_va_t *p_external, picture_t *p_picture, AVFrame *p_ff )
>> +{
>> +    VLC_UNUSED( p_ff );
>> +    vlc_va_vda_t *p_va = vlc_va_vda_Get( p_external );
>> +
>> +    if ( !p_va->top_frame )
>> +    {
>> +        msg_Dbg( p_va->p_log, "Decoder is buffering...");
>> +        return VLC_EGENERIC;
>> +    }
>> +
>> +    CVPixelBufferRef cv_buffer = p_va->top_frame->cv_buffer;
>> +    vda_CopyCVBuffer( p_picture, cv_buffer );
>> +
>> +    return VLC_SUCCESS;
>> +}
>> +
>> +static void Release( vlc_va_t *p_external, AVFrame *p_ff )
>> +{
>> +    VLC_UNUSED( p_ff );
>> +    VLC_UNUSED( p_external );
>> +}
> 
> I am a bit dubious about the way you implemented the surface
> attachment to the picture.
> To have it correctly working you should attach the correct surface to its
> associated AVFrame buffer. Otherwise I don't think the picture order will
> be the correct one.

The surface is output by the vda decoder via a callback. The picture's date can
be overridden by the current surface pts ?

> 
>> […]
>> +static void Delete( vlc_va_t *p_external )
>> +{
>> +    vlc_va_vda_t *p_va = vlc_va_vda_Get( p_external );
>> +    Close( p_va );
>> +}
> You could merge Delete() and Close().

Done.

> 
>> +vlc_va_t *vlc_va_NewVDA( vlc_object_t *p_log, int i_codec_id, void *p_extra, int i_extra )
> Do you really need the extra data at this point?
> You won't always have them at that point.

Yes, cause of vda API requires extra data to create the decoder.

>> +{
>> +    if ( i_codec_id != CODEC_ID_H264 )
>> +        return NULL;
>> +
>> +    if ( !p_extra || i_extra < 7 )
>> +    {
>> +        msg_Warn( p_log, "VDA requires extradata." );
>> +        return NULL;
>> +    }
>> +
>> +    vlc_va_vda_t *p_va = calloc( 1, sizeof(*p_va) );
>> +    if( !p_va )
>> +        return NULL;
>> +
>> +    msg_Dbg( p_log, "Trying VDA module" );
> I don't think this log is helpful.

I moved it just before the call which initializes the vda module.

>> […]
>> -#if defined(HAVE_AVCODEC_VAAPI) || defined(HAVE_AVCODEC_DXVA2)
>> -    add_bool( "ffmpeg-hw", false, HW_TEXT, HW_LONGTEXT, false )
>> +#if defined(HAVE_AVCODEC_VAAPI) || defined(HAVE_AVCODEC_DXVA2) || defined(HAVE_AVCODEC_VDA)
>> +    add_bool( "ffmpeg-hw", true, HW_TEXT, HW_LONGTEXT, true )
>> #endif
>> #if defined(FF_THREAD_FRAME)
>>     add_integer( "ffmpeg-threads", 0, THREADS_TEXT, THREADS_LONGTEXT, true );
>> -- 
>> 1.7.5.4
> The check against HAVE_AVCODEC_VDA should be merged in one of the
> previous patch.
> I don't think changing the default value is a good idea, at least for now.

Done. I could provide a patch to add this option to the macosx gui.


Best regards,
Sebastien

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-avcodec-Mac-OS-X-Video-Acceleration-VDA-support.patch
Type: application/octet-stream
Size: 14787 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20111113/afa88345/attachment.obj>
-------------- next part --------------