[vlc-devel] [PATCH] Adding caf file demuxer module.

[Jean-Baptiste Kempf]

On 24 Sep, Matthias Keiser wrote :
> TODO:
> 
> - channel layout
> - 64 bit float LPCM is broken (sound has artifacts with little endian, only silence plays for big endian). I didn't investigate why. I honestly don't know if such files are even in the wild.

You should add a TODO in comment in the code.

> +#ifdef HAVE_CONFIG_H
> +# include "config.h"
> +#endif
> +#include <math.h>
> +#include <limits.h>

You probably miss a -lm link.

> +#include <vlc_common.h>
> +#include <vlc_plugin.h>
> +#include <vlc_demux.h>
> +#include <vlc_codecs.h>
> +//#include <vlc_config.h>

Remove the unused one.

> +static int ParseVarLenInteger( const uint8_t *p_buff, uint64_t i_buff_len, int64_t *pi_value_out, int32_t *i_len_out )
> +{
> +    *i_len_out = 0;
> +
> +    uint64_t i_value = 0;
> +    bool finished = false;
> +
> +    for( uint32_t i = 0; i < i_buff_len; i++ )
> +    {
> +        if( (( i_value >> 32 ) << 7 ) > INT32_MAX )
> +        {
> +            return VLC_EGENERIC; /* overflow */
> +        }
> +        uint8_t i_byte = p_buff[i];
> +        i_value = ( i_value << 7 ) | ( i_byte & 0x7f );
> +        
> +        ( *i_len_out )++;
> +        
> +        if( !( i_byte & 0x80 ))
> +        {
> +            finished = true;
> +            break;
> +        }
> +    }
> +    
> +    if( !finished )
> +    {
> +        return VLC_EGENERIC;
> +    }
> +    
> +    *pi_value_out = i_value;
> +    
> +    return VLC_SUCCESS;
> +}

Remove trailing space.

> +/* Utility function that reads a big endian double from the input buffer. */
> +
> +static inline double GetDBLBE( const uint8_t *p )
> +{
> +    union {
> +        uint64_t uint64;
> +        double dbl;
> +    } u_64;
> +    
> +    u_64.uint64 = GetQWBE( p );
> +    return u_64.dbl;
> +}

This should go in the core, but fair enough for now.

> +static int64_t TotalNumFrames( demux_t *p_demux )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +    
> +    if( !NeedsPacketTable( p_sys ))
> +    {
> +        int64_t i_data_size;
> +        
> +        if( p_sys->i_data_size >= 0 )
> +        {
> +            i_data_size = p_sys->i_data_size;
> +        }
> +        else
> +        {
> +            i_data_size = stream_Size( p_demux->s ) - p_sys->i_data_offset;
> +        }

Maybe using ?: ternary operator would be nicer. Maybe not :)

> +static int64_t TotalNumSamples( demux_t *p_demux )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +    
> +    if( !NeedsPacketTable( p_sys ))
> +    {
> +        return TotalNumFrames( p_demux ) * p_sys->fmt.audio.i_frame_length;
> +    }
> +    else
> +    {
> +        return p_sys->packet_table.i_num_valid_frames + p_sys->packet_table.i_num_priming_frames + p_sys->packet_table.i_num_remainder_frames;

Wrap your line...

> +static inline vlc_fourcc_t Read4CC( const uint8_t *p )
> +{
> +    
> +    return VLC_FOURCC( p[0], p[1], p[2], p[3] );
> +}

You shouldn't call it 4CC, but FOURCC

> +static int SetSpanWithSample( demux_t *p_demux, frame_span_t *span, int64_t i_target_sample )
> +{
> +    demux_sys_t *p_sys = p_demux->p_sys;
> +    
> +    int64_t i_num_frames = TotalNumFrames( p_demux );
> +    
> +    if( !NeedsPacketTable( p_sys ))
> +    {
> +        int64_t i_frame = i_target_sample / p_sys->fmt.audio.i_frame_length;
> +        int64_t i_remaining = i_target_sample - i_frame * p_sys->fmt.audio.i_frame_length;
> +        if( i_remaining > ( p_sys->fmt.audio.i_frame_length / 2 ))
> +            i_frame++;
> +        
> +        if( i_frame > i_num_frames )
> +            i_frame = i_num_frames;
> +        
> +        span->i_frames = i_frame;
> +        span->i_samples = i_frame * p_sys->fmt.audio.i_frame_length;
> +        span->i_bytes = i_frame * p_sys->fmt.audio.i_bytes_per_frame;
> +        span->i_desc_bytes = 0;
> +    }
> +    else {

Wrong style

> +        
> +        memset( span, 0, sizeof( frame_span_t ));

Why memset here?

> +    if( i_fmt == VLC_FOURCC( 'l', 'p', 'c', 'm' ))

VLC_CODEC_DVD_LPCM

> +    p_sys->fmt.p_extra = malloc( i_extra );

Check malloc usage. Always.

Remove trailing spaces...


Best regards,

-- 
Jean-Baptiste Kempf
http://www.jbkempf.com/ - +33 672 704 734
Sent from my Electronic Device