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

Måns Rullgård mans
Fri Feb 22 01:14:11 CET 2008


Nico Sabbi <Nicola.Sabbi at poste.it> writes:

> Il Tuesday 19 February 2008 01:26:38 M?ns Rullg?rd ha scritto:
>
>> 
>> You could move p += program_info_length statements (this one and the
>> one below) just after the while loop.  Adding zero will do no harm.
>
> done
>
>> 
>> > +            break;
>> > +        }
>> > +        tag = get8(&p, p_end);
>> > +        len = get8(&p, p_end);
>> > +        if(len == -1 || len > program_info_length - 2) {
>> 
>> You need to declare len as int to compare it to -1.  Like this, a
>> return value of -1 will turn into a seemingly valid 255.
>
> done
>
>> 
>> As I've said, I don't like the way this looks, neither before nor
>> after the patch.  Unfortunately, I can't think of an easy way of
>> cleaning it up, and I'm working on a better ts demuxer anyway.
>> 
>
> I see, but until you have finished porting your demuxer may I commit it?
>
> Index: libavformat/mpegts.c
> ===================================================================
> --- libavformat/mpegts.c	(revisione 12129)
> +++ libavformat/mpegts.c	(copia locale)
> @@ -31,6 +31,7 @@
>  /* maximum size in which we look for synchronisation if
>     synchronisation is lost */
>  #define MAX_RESYNC_SIZE 4096
> +#define REGISTRATION_DESCRIPTOR 5
>  
>  typedef struct PESContext PESContext;
>  
> @@ -478,6 +479,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;
>  
>  #ifdef DEBUG_SI
>      av_log(ts->stream, AV_LOG_DEBUG, "PMT: len %i\n", section_len);
> @@ -505,6 +507,30 @@
>      program_info_length = get16(&p, p_end) & 0xfff;
>      if (program_info_length < 0)
>          return;
> +    while(program_info_length > 0) {
> +        uint8_t tag;
> +        int len;
> +        if(program_info_length < 2)
> +            //something is broken, exit the program_descriptors_loop
> +            break;

This can be simplified as while(program_info_length >= 2) ...
Yes, I should have spotted this last time around.

> +        tag = get8(&p, p_end);
> +        len = get8(&p, p_end);
> +        if(len == -1 || len > program_info_length - 2)
> +            //something else is broken, exit the program_descriptors_loop
> +            break;

Again, sorry for not realising this sooner, but reading tag and len
will always succeed since program_info_length is at least 2 here. so
there is no need to check for len == -1.  We still need to check for
len > program_info_length-2, of course.

> +        program_info_length -= len + 2;
> +        if(tag == REGISTRATION_DESCRIPTOR && len >= 4) {
> +            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')

Maybe break this line.

> +                has_hdmv_descr = 1;
> +        }
> +        p += len;
> +    }
>      p += program_info_length;
>      if (p >= p_end)
>          return;

Apart from the comments above, the patch is OK.

-- 
M?ns Rullg?rd
mans at mansr.com




More information about the ffmpeg-devel mailing list