[vlc-devel] Adding Cook support in mkv - needs review

[Denis Charmet]

Le mercredi 04 juillet 2012 à 12:39:22, Jean-Baptiste Kempf a écrit :
> On Wed, Jul 04, 2012 at 12:27:27AM +0200, Denis Charmet wrote :
> > +#include <arpa/inet.h>
> 
> Sure?
> 
I wanted htons and that's what man htons suggests, I didn't look for vlc
macro.

> > +                    p_tk->p_subpackets = (block_t**) calloc( p_tk->i_subpackets, sizeof(block_t*));
> No check for NULL?
>
Yeah my bad, it was originally a quick proof of concept, I'll add the
check.

> > +        if( tk->fmt.i_cat == VIDEO_ES || tk->fmt.i_cat == AUDIO_ES )
> >          tk->i_last_dts = p_block->i_dts;
> Needs alignment?
>

Will be done.

> > -        /* The blocks are in coding order so we can safely consider that only references are in chronological order */
> > -        if( p_sys->i_pts > p_sys->i_pcr + 300000 )
> > +        mtime_t i_pcr = VLC_TS_INVALID;
> > +        for( size_t i = 0; i < p_segment->tracks.size(); i++)
> > +            if( p_segment->tracks[i]->i_last_dts > VLC_TS_INVALID &&
> > +                ( p_segment->tracks[i]->i_last_dts < i_pcr || i_pcr == VLC_TS_INVALID ))
> > +                i_pcr = p_segment->tracks[i]->i_last_dts;
> 
> Hmm, Laurent?
>
That's the most tricky part for me but it's the only way I found to make
it work. It's done that way in real.c and other demuxes, cook's
behaviour messes up the traditionnal mkv timings.

> > +    /* real audio */
> > +    uint32_t i_sub_packet_h;
> > +    uint32_t i_frame_size;
> > +    uint32_t i_subpacket_size;
> > +    block_t  **p_subpackets;
> > +    size_t   i_subpackets;
> > +    size_t   i_subpacket;
> 
> Why not a sub-struct?
> 
Because I was lazy, I can do it, now do I create a void * p_sys to
generalize or do I let the substructure in every track?

Thanks for the review.

-- 
TypX
Le mauvais esprit est un art de vivre