[vlc-devel] [PATCH 2/6] codec: use macros to avoid confusion between data[0] and data[3] types

[Steve Lhomme]

On Thu, Oct 20, 2016 at 3:40 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le torstaina 20. lokakuuta 2016, 15.26.09 EEST Steve Lhomme a écrit :
>> having data[3] = data[0] doesn't help
>
> It does because it matches the libavcodec documetation. This patch is making
> things even more confusing.

I only found the AV_PIX_FMT_xxx_VLD that explains what data[3] should
be and only the AV_PIX_FMT_VDPAU_xxx describe that data[0] should be a
vdpau_render_state which we don't seem to use.

I trust that data[0] must not be NULL according to the VLC
documentation but changing the type had no effect for me at all. It
would be weird that libavcodec knows what a picture_t is anyway.

AV_FRAME_SURFACE() may be renamed to AV_FRAME_SURFACE_CTX() to avoid
more confusion. But the change really helped me decipher what's going
on in there.

> If you don´t like libavcodec name space, vlc-devel is really not the place to
> fix, or rather, clobber it.
>
>> ---
>>  modules/codec/avcodec/video.c | 17 ++++++++++-------
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/modules/codec/avcodec/video.c b/modules/codec/avcodec/video.c
>> index ff7a046..4ae22d6 100644
>> --- a/modules/codec/avcodec/video.c
>> +++ b/modules/codec/avcodec/video.c
>> @@ -43,6 +43,11 @@
>>  #include "avcodec.h"
>>  #include "va.h"
>>
>> +/* data[0] must be non-NULL for libavcodec internal checks.
>> + * data[3] actually contains the format-specific surface handle. */
>> +#define AV_FRAME_SURFACE(f)       (f->data[0])
>> +#define AV_FRAME_SURFACE_DATA(f)  (f->data[3])
>> +
>>  /**************************************************************************
>> *** * decoder_sys_t : decoder descriptor
>>
>> ***************************************************************************
>> **/ @@ -971,7 +976,7 @@ static picture_t *DecodeVideo( decoder_t *p_dec,
>> block_t **pp_block ) else
>>          {
>>              if( p_sys->p_va != NULL )
>> -                vlc_va_Extract( p_sys->p_va, p_pic, frame->data[3] );
>> +                vlc_va_Extract( p_sys->p_va, p_pic,
>> AV_FRAME_SURFACE_DATA(frame) ); picture_Hold( p_pic );
>>          }
>>
>> @@ -1118,21 +1123,19 @@ static int lavc_va_GetFrame(struct AVCodecContext
>> *ctx, AVFrame *frame, decoder_t *dec = ctx->opaque;
>>      vlc_va_t *va = dec->p_sys->p_va;
>>
>> -    if (vlc_va_Get(va, pic, &frame->data[0]))
>> +    if (vlc_va_Get(va, pic, &AV_FRAME_SURFACE(frame)))
>>      {
>>          msg_Err(dec, "hardware acceleration picture allocation failed");
>>          picture_Release(pic);
>>          return -1;
>>      }
>> -    /* data[0] must be non-NULL for libavcodec internal checks.
>> -     * data[3] actually contains the format-specific surface handle. */
>> -    frame->data[3] = frame->data[0];
>> +    AV_FRAME_SURFACE_DATA(frame) = AV_FRAME_SURFACE(frame);
>>
>>      void (*release)(void *) = va->release;
>>      if (va->release == NULL)
>>          release = lavc_ReleaseFrame;
>>
>> -    frame->buf[0] = av_buffer_create(frame->data[0], 0, release, pic, 0);
>> +    frame->buf[0] = av_buffer_create(AV_FRAME_SURFACE(frame), 0, release,
>> pic, 0); if (unlikely(frame->buf[0] == NULL))
>>      {
>>          release(pic);
>> @@ -1140,7 +1143,7 @@ static int lavc_va_GetFrame(struct AVCodecContext
>> *ctx, AVFrame *frame, }
>>
>>      frame->opaque = pic;
>> -    assert(frame->data[0] != NULL);
>> +    assert(AV_FRAME_SURFACE(frame) != NULL);
>>      return 0;
>>  }
>
>
> --
> Rémi Denis-Courmont
> Nonsponsored VLC developer
> http://www.remlab.net/CV.pdf
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel