[vlc-devel] [PATCH 2/2] codec/dirac: Rewrite libdirac(-research) encoding support

[David Flynn]

On 2008-11-10, Laurent Aimar <fenrir at via.ecp.fr> wrote:
>>      if( ( p_sys = (encoder_sys_t *)malloc(sizeof(encoder_sys_t)) ) == NULL )
>  Useless cast in C. Using sizeof(*p_sys) may be safer.
>>      memset( p_sys, 0, sizeof(encoder_sys_t) );
>  ... missing * or use *p_sys or use calloc iof the above malloc.

Next time i rewrite a module, please remind me to delete the ENTIRE
thing first.  a large number of these 'faults' pre existed.  I'm seeing
this over and over again.

FIXED

>  You MUST check the frame rate validity (x/0 is allowed when the frame rate
> information is not availaible or N/A).
>  Same a bit later for date_Init.

Anything without frame rate set is rejected now in previous patch

FIXED

>  Some parameters seem not initialied to a valid value if !val.psz_string or
> val.psz_string is not a valid value.
> (Either handle this case as I420 or as a fatal error).

Strings were all fixed as part of the formatting change.

FIXED

>> +        if( !p_sys->p_dirac )
>> +        {
>> +            msg_Err( p_enc, "Failed to initialize dirac encoder\n" );
>> +            p_enc->b_error = 1;
> I think it will segfault after that as p_dirac is NULL.

It didn't, but for the wrong reason -- the dirac library checks for a
valid handle and returns an error causing a return NULL from Encode()

FIXED

>> +    p_block = block_New( p_enc, 1 );
>> +    p_block->i_dts = p_pic->date - p_sys->i_pts_offset;
>  p_block MUST be checked.

I'll call abort() then? It does seem somewhat rediculously pedantic
to continually check for memory allocations in areas where there is no
ability to recover.  If you can't allocate the few hundred bytes for
that block, VLC really is going to explode a few seconds later --
something made far worse by things like overcommit.

Bring back exceptions, all is forgiven.

FIXED (set b_error, return NULL, give up and hope the world doesn't end)

..david