[vlc-devel] [PATCH] x264: write first SEI NAL into first picture

[Laurent Aimar]

Hi,

On Thu, Nov 04, 2010 at 05:30:09PM +0100, Rafaël Carré wrote:
> It contains the x264 version info and options used for encoding
> If we let it in extradata it will not be present in the final file
> 
> libavcodec does that already, and this makes it easier to compare x264
> options used by VLC and FFmpeg.
> 
> Comparing FFmpeg and VLC with the exact same set of options might help
> finding why mpeg (at least mpeg2/mpeg4/flv) encoding is 5 times slower
> with vlc than with ffmpeg, supposedly for the same set of options.
> 
>  #ifdef PTW32_STATIC_LIB
> @@ -753,6 +755,8 @@ static int  Open ( vlc_object_t *p_this )
>          return VLC_ENOMEM;
>      p_sys->i_initial_delay = 0;
>      p_sys->psz_stat_name = NULL;
> +    p_sys->i_sei_size = 0;
> +    p_sys->p_sei = 0;
>  
>      x264_param_default( &p_sys->param );
>      char *psz_preset = var_GetString( p_enc, SOUT_CFG_PREFIX  "preset" );
> @@ -1270,8 +1274,23 @@ static int  Open ( vlc_object_t *p_this )
>      uint8_t *p_tmp = p_enc->fmt_out.p_extra;
>      for( i = 0; i < i_nal; i++ )
>      {
> -        memcpy( p_tmp, nal[i].p_payload, nal[i].i_payload );
> -        p_tmp += nal[i].i_payload;
> +        if( nal[i].i_type != NAL_SEI )
> +        {
> +            memcpy( p_tmp, nal[i].p_payload, nal[i].i_payload );
> +            p_tmp += nal[i].i_payload;
> +        }
> +        else
> +        {
> +            p_enc->fmt_out.i_extra -= nal[i].i_payload;
 I find it a bit dangerous, it might be better to store the final size only
after the current loop.
> +            p_sys->i_sei_size = nal[i].i_payload;
> +            p_sys->p_sei = malloc( p_sys->i_sei_size );
> +            if( !p_sys->p_sei )
> +            {
> +                Close( VLC_OBJECT(p_enc) );
> +                return VLC_ENOMEM;
> +            }
> +            memcpy( p_sys->p_sei, nal[i].p_payload, nal[i].i_payload );
> +        }
 It won't work correctly if they are multiples SEI.
>      }

> @@ -1324,6 +1343,9 @@ static block_t *Encode( encoder_t *p_enc, picture_t *p_pict )
>      for( i = 0, i_out = 0; i < i_nal; i++ )
>          i_out += nal[i].i_payload;
>  
> +    if( p_sys->i_sei_size )
 No need to add the test.
> +        i_out += p_sys->i_sei_size;

>      p_block = block_New( p_enc, i_out );
>      if( !p_block ) return NULL;
>  
> @@ -1333,6 +1355,14 @@ static block_t *Encode( encoder_t *p_enc, picture_t *p_pict )
>          memcpy( p_block->p_buffer + i_out, nal[i].p_payload, nal[i].i_payload );
>          i_out += nal[i].i_payload;
>      }
> +    if( p_sys->i_sei_size )
> +    {
> +        memcpy( p_block->p_buffer, p_sys->p_sei, p_sys->i_sei_size );
> +        i_out += p_sys->i_sei_size;
> +        p_sys->i_sei_size = 0;
> +        free( p_sys->p_sei );
> +        p_sys->p_sei = NULL;
> +    }
 You cannot add SEI at the end of an access unit. You must respect the order
requested by the h264 norm. Basically, to be safe, you should put them after
any SEI presents in the current bitstream but before any slices NAL (I suppose those
globals SEI you want to add do not contains any "buffering period SEI message").

Regards,

-- 
fenrir