[FFmpeg-devel] [PATCH] a couple of updates for MPEGTS

[Nico Sabbi]

Il Sunday 17 February 2008 15:49:05 M?ns Rullg?rd ha scritto:
> Nico Sabbi <Nicola.Sabbi at poste.it> writes:
> 
> > Il Tuesday 12 February 2008 23:36:59 Nico Sabbi ha scritto:
> >> 
> >> M?ns Rullg?rd wrote:
> >> >> In incoming there are a couple of interesting samples: one is a trailer
> >> >> of resident evil that uses 0x82 as stream_type to identify DCA
> >> >> (I don't know in what system this mapping is defined) ;
> >> 
> >> >I'd obviously like to know what system uses this value.  The sample
> >> >has a Bluray registration descriptor, so it maybe that is where it
> >> >comes from.  The PS3 refuses to recognise any audio in the file, but
> >> >that need not mean anything.
> >> >
> >> >Otherwise I have only the standard objection about non-standard
> >> >features.  Randomly assigning codes in the private range to codecs
> >> >will sooner or later lead to a clash.  In this case, the registration
> >> >descriptor could probably be used to identify the codec mapping in a
> >> >safer manner.  It's hard to say for sure without access to the Bluray
> >> >spec.
> >> >
> >> >Given the amount of work required to implement this properly, and that
> >> >we so far only have this sample using stream type 0x82, I reckon we
> >> >can still add this to the list.
> >> 
> >> I'll post tomorrow the patch to parse the program descriptors.
> >> let's make it better now and don't regret in the future
> >
> > Index: libavformat/mpegts.c
> > ===================================================================
> > --- libavformat/mpegts.c	(revisione 12129)
> > +++ libavformat/mpegts.c	(copia locale)
> > @@ -478,6 +478,7 @@
> >      int desc_list_len, desc_len, desc_tag;
> >      int comp_page = 0, anc_page = 0; /* initialize to kill warnings */
> >      char language[4] = {0}; /* initialize to kill warnings */
> > +    int has_hdmv_descr = 0;
> 
> I have an instinctive dislike for things like this.  They tend to
> accumulate and become unmanageable.
> 
> >  #ifdef DEBUG_SI
> >      av_log(ts->stream, AV_LOG_DEBUG, "PMT: len %i\n", section_len);
> > @@ -505,7 +506,28 @@
> >      program_info_length = get16(&p, p_end) & 0xfff;
> >      if (program_info_length < 0)
> >          return;
> > -    p += program_info_length;
> > +    while(program_info_length > 0) {
> > +        uint8_t tag, len;
> > +        tag = get8(&p, p_end);
> > +        len = get8(&p, p_end);
> > +        if(len < 0) {
> 
> This can never happen.  You (correctly) declared len as unsigned.
> 
> > +            //something is broken, exit the program_descriptors_loop
> > +            p += program_info_length;
> > +            break;
> > +        }
> > +        program_info_length -= len + 2;
> > +        if(tag == 0x5 && len>=4) {
> 
> I'd prefer if that 5 were #defined with a descriptive name.
> 
> > +            uint8_t bytes[4];
> > +            bytes[0] = get8(&p, p_end);
> > +            bytes[1] = get8(&p, p_end);
> > +            bytes[2] = get8(&p, p_end);
> > +            bytes[3] = get8(&p, p_end);
> > +            len -= 4;
> > +            if(bytes[0] == 'H' && bytes[1] == 'D' && bytes[2] == 'M' && bytes[3] == 'V')
> > +                has_hdmv_descr = 1;
> > +        }
> > +        if(len > 0) p += len;
> 
> Useless if() since len is unsigned.
> 
> > +    }
> >      if (p >= p_end)
> >          return;
> >      for(;;) {
> > @@ -571,6 +593,9 @@
> >          }
> >          p = desc_list_end;
> >  
> > +        if(stream_type == 0x82 && has_hdmv_descr)
> > +            stream_type = STREAM_TYPE_AUDIO_DTS;
> 
> While this will of course work, I don't like it.  The stream_type
> value should not be altered based on the registration (or other)
> descriptor; only the interpretation should change.
> 

updated with some more security check on "len", but I didn't
find a way to handle differently the presence of the hdmv.
Please, explain
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dts-reg3.diff
Type: text/x-diff
Size: 2786 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080217/1f0f1481/attachment.diff>