[vlc-devel] [PATCH] Allow hevc muxing in mp4

[Denis Charmet]

Hi,

Le lundi 24 février 2014 à 10:01:19, Rafaël Carré a écrit :
> >              /*Get fps from vps if available and not already forced*/
> > -            if( p_sys->f_fps == 0.0f && ( p_block_out->p_buffer[3] & 0x7E ) == 0x40 )
> > +            if( p_sys->f_fps == 0.0f && ( p_block_out->p_buffer[4] & 0x7E ) == 0x40 )
> 
> No need to check if block is large enough here? (i_buffer >= 5)
> 
Nope the packetizer returns i_au_min_size packets which is set to 5.
> >              {
> >                  if( getFPS( p_demux, p_block_out) )
> >                  {
> > @@ -195,9 +196,9 @@ static int Demux( demux_t *p_demux)
> >              }
> >  
> >              /* Update DTS only on VCL NAL*/
> > -            if( ( p_block_out->p_buffer[3]&0x7E ) < 0x40 && p_sys->f_fps )
> > +            if( ( p_block_out->p_buffer[4]&0x7E ) < 0x40 && p_sys->f_fps )
> 
> Maybe p_buffer[4] & 0x7e should be cached.
> 
'k
> >              {
> > -                es_out_Control( p_demux->out, ES_OUT_SET_PCR, p_sys->i_dts );
> > +                es_out_Control( p_demux->out, ES_OUT_SET_PCR, VLC_TS_0 + p_sys->i_dts );
> 
> Initialize i_dts to VLC_TS_0 instead ?
> 
Already done locally.
> >  static void CreateDecodedNAL( uint8_t **pp_ret, int *pi_ret,
> >                                const uint8_t *src, int i_src )
> >  {
> > -    const uint8_t *end = &src[i_src];
> >      uint8_t *dst = malloc( i_src );
> >      if( !dst )
> >          return;
> >  
> >      *pp_ret = dst;
> 
> Make CreateDecodedNal return uint8_t * instead?
>
Yeah as previously stated this was a rip-off the h264 demux

> > +            if (p_stream->fmt.i_codec == VLC_CODEC_H264 ||
> > +                p_stream->fmt.i_codec == VLC_CODEC_HEVC)
> >                  p_data = ConvertAVC1(p_data);
> 
> Maybe function name should be changed?
>
If you have one, I'm taking it.
 
> > +/*
> > +The following table comes from iso/iec 14496-15 3rd edition
> > +
> > [...]
> > +*/
> 
> Is this description your own work ?
> 
As stated in the comment no it's not.

> > +
> > +static void hevcParseVPS(uint8_t * p_buffer, size_t i_buffer, uint8_t *general,
> > +                         uint8_t * numTemporalLayer, bool * temporalIdNested)
> > +{
> > +    const size_t i_decoded_nal_size = 512;
> 
> Magic value?
> 
Big enough value, I'm too lazy to actually calculate what's the worst
case with the 0x03 emulation bytes and all the flags... And this
prevents malloc.
> > +    uint8_t p_dec_nal[i_decoded_nal_size];
> > +    size_t i_size = (i_buffer < i_decoded_nal_size)?i_buffer:i_decoded_nal_size;
> > +    nal_decode(p_buffer, p_dec_nal, i_size);
> > +
> > +    /* first two bytes are the NAL header 3rd and 4th are:
> 
> Comma before "3rd" ?
> 
Srsly?

> > +    if (p_stream->fmt.i_extra > 0) {
> 
> if (!p_stream->fmt.i_extra)
> 	return hvcC;
> 
pfff ok

Regards,

-- 
Denis Charmet - TypX
Le mauvais esprit est un art de vivre