[FFmpeg-devel] [PATCH][7/8] Add VA API accelerated H.264 decoding (take 4)

[Michael Niedermayer]

On Mon, Feb 09, 2009 at 02:36:14PM +0100, Gwenole Beauchesne wrote:
> On Fri, 6 Feb 2009, Michael Niedermayer wrote:
> 
> >> +static int vaapi_h264_fill_picture(VAPictureH264 *va_pic, Picture *pic, int pic_structure)
> >> +{
> >> +    struct vaapi_render_state *rds;
> >> +    int poc;
> >> +
> >> +    assert(va_pic != NULL);
> >> +    if (va_pic == NULL)
> >> +        return -1;
> >> +    assert(pic != NULL);
> >> +    if (pic == NULL)
> >> +        return -1;
> >
> > and please decide if the condition is allowed or not and then remove either
> > the assert or the if()
> 
> The assert() is useful for debugging and early abort. Otherwise, if built 
> with NDEBUG, this would get un-noticed in case code is changed later.

as ive said, either the condition is allowed or it is not.
If its not, removing the return ensures that the code will end either by
assert of segfault of a read near NULL
If it is allowed the assert() is a bug and the near complete lack of
any check for the return value of vaapi_h264_fill_picture() gives me
no faith that you have even considered what effect the return might have

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090209/50fcb966/attachment.pgp>