[FFmpeg-devel] New patch for mpegts.c

[Michael Niedermayer]

On Tue, Oct 16, 2012 at 01:31:35PM +0100, JULIAN GARDNER wrote:
> 
> 
> Whilst working on some new additions to MPEGTS support i found that the current code did not take into account the "Table Version Number", which caused FFMPEG to process every table that was in the TS. After adding the new code and validating against a 3 minute TS the counts on tables processed went down from 1360 to 3.

can you provide a testcase that shows the improvment?


[...]
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 33a0eaf..de06a68 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -58,7 +58,18 @@ typedef struct MpegTSPESFilter {
>      void *opaque;
>  } MpegTSPESFilter;
>  
> -typedef void SectionCallback(MpegTSFilter *f, const uint8_t *buf, int len);
> +typedef struct SectionHeader {
> +    uint8_t tid;
> +    uint16_t id;
> +    uint8_t version;
> +    uint8_t sec_num;
> +    uint8_t last_sec_num;
> +} SectionHeader;

moving of code if its needed, belongs in a seperate patch


> +
> +static int parse_section_header(SectionHeader *h,
> +                                const uint8_t **pp, const uint8_t *p_end);
> +
> +typedef void SectionCallback(MpegTSFilter *f, const uint8_t *buf, const uint8_t *buf_end, SectionHeader *h);
>  
>  typedef void SetServiceCallback(void *opaque, int ret);
>  
> @@ -76,11 +87,14 @@ struct MpegTSFilter {
>      int pid;
>      int es_id;
>      int last_cc; /* last cc code (-1 if first packet) */
> +    uint8_t last_version;
>      enum MpegTSFilterType type;
>      union {
>          MpegTSPESFilter pes_filter;
>          MpegTSSectionFilter section_filter;
>      } u;
> +    const char *name;
> +    uint8_t tid;
>  };
>  
>  #define MAX_PIDS_PER_PROGRAM 64
> @@ -318,18 +332,45 @@ static void write_section_data(AVFormatContext *s, MpegTSFilter *tss1,
>              }else
>                  crc_valid = 2;
>          }
> -        if (crc_valid)
> -            tss->section_cb(tss1, tss->section_buf, tss->section_h_size);
> +        if (crc_valid) {
> +            const uint8_t *p_end;
> +            const uint8_t *p;
> +            SectionHeader h;
> +
> +            av_dlog(ts->stream, "%s: len %i\n", tss1->name, tss->section_h_size);
> +            hex_dump_debug(ts->stream, tss->section_buf, tss->section_h_size);
> +
> +            p = tss->section_buf;
> +            p_end = p + tss->section_h_size - 4;
> +
> +            if (parse_section_header(&h, &p, p_end) < 0)
> +                return;
> +
> +            if (h.tid != tss1->tid)
> +                return;

changing tid and parse_section_header handling belongs to 2 seperate
patches (and why are these changes needed at all?)


> +
> +            // Check Version Numbers
> +            if (tss1->last_version==h.version)
> +                return;
> +            tss1->last_version = h.version;

the last_version should only be updated after successfull parsing of
the data
also how does this interact when things are split into multiple
sections?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There seems to be only one solution to NIH syndrom, ... a shooting squad
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121016/672d2393/attachment.asc>