Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#10794 closed Patch - Bug Fix (fixed)

[PATCH] ffmpeg: Dont discard PES sections split across a TS packet

Reported by: Lawrence Rust <lvr@…> Owned by: danielk
Priority: minor Milestone: 0.26
Component: MythTV - DVB Version: 0.25-fixes
Severity: medium Keywords: MHEG 'red button'
Cc: Ticket locked: no


In libavformat/mpegts.c, if a TS packet has a pointer field which marks the boundary of a PES section then the first section will be discarded if a second section is started in the remainder of the TS packet.

This fix adds a check for a completed section and if present, appends the 2nd section table to the 1st.

This bug is most noticeable on the BBC MHEG text (red button) service which often will not start because the boot application or essential files have missing blocks. Currently, it is often impossible to view the BBC 'red button' service on FreeSat? because of this bug.

The attached patch is for fixes/0.25 and is also applicable to fixes/0.24. The same bug also affects git master but ffmpeg has been updated since and has changed considerably in this area.

I take it that this bug has been around for a long time and has caused problems for text and subtitle users for a similarly long time.

The bug and fix can be tested on this sample (16MB) recorded from Freesat BBC1 on the 25-May with mythtv fixes/0.24:

Attachments (1)

0001-ffmpeg-Dont-discard-PES-sections-split-across-a-TS-p.patch (2.7 KB) - added by Lawrence Rust <lvr@…> 8 years ago.

Download all attachments as: .zip

Change History (6)

Changed 8 years ago by Lawrence Rust <lvr@…>

comment:1 Changed 8 years ago by danielk

Milestone: unknown0.26

Cool, I was aware of this deficiency but didn't realise it was causing any real world problems.

comment:2 Changed 8 years ago by dekarl@…

Looks like the patch should apply to our renamed mpegts-mythtv.c on master, too.

We could ask for real world issues over at MumuDVB, they have rewritten their section handler completely.

This is a important piece of MuMuDVB since it's needed by autoconfiguration (and other pieces of code)

Their autoconfiguration correspondents to our DVB channel scanner which has some interesting issues, too. (thinking of #10217, which could use retesting after this patch gets commited. Though #10054 might already have fixed it.)

comment:3 Changed 8 years ago by beirdo

Please note: from 0.26-pre onwards, this is mpegts-mythtv.c

Please do not mess with mpegts.c in ffmpeg at all past the creation of our own version unless absolutely necessary.

This is a perfect example of why you should be developing on master, not on old branches. Whether danielk applies this to fixes/0.25 is, of course, his decision to make :)

comment:4 Changed 8 years ago by Lawrence Rust <lvr@…>

Resolution: fixed
Status: newclosed

In ac1fd96ac762f5081fb399d8838dc34c30d7630d/mythtv:

Fixes #10794. Keep more sections in mpegts-mythtv.c

Signed-off-by: Daniel Kristjansson <danielk@…>

comment:5 Changed 8 years ago by Lawrence Rust <lvr@…>

In c353a84ea3398ca9b8d39c33f0247e86b1fb7797/mythtv:

Refs #10794. Add some debugging for failed CRC.

Signed-off-by: Daniel Kristjansson <danielk@…>

Note: See TracTickets for help on using tickets.