Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 23 months ago

Closed 21 months ago

Last modified 21 months 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

Description

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:
ftp://guest@ftp.softsystem.co.uk/nomheg.mpg

Attachments (1)

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

Download all attachments as: .zip

Change History (6)

Changed 23 months ago by Lawrence Rust <lvr@…>

comment:1 Changed 23 months ago by danielk

  • Milestone changed from unknown to 0.26

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

comment:2 Changed 23 months 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. http://mumudvb.braice.net/mumudrupal/node/56

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 22 months 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 21 months ago by Lawrence Rust <lvr@…>

  • Resolution set to fixed
  • Status changed from new to closed

In ac1fd96ac762f5081fb399d8838dc34c30d7630d/mythtv:

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

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

comment:5 Changed 21 months ago by Lawrence Rust <lvr@…>

In c353a84ea3398ca9b8d39c33f0247e86b1fb7797/mythtv:

Refs #10794. Add some debugging for failed CRC.

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

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.