Opened 13 years ago

Closed 13 years ago

#1899 closed patch (fixed)

frontend segfault (dvb teletext subtitle related?)

Reported by: torbjorn.jansson@… Owned by: danielk
Priority: minor Milestone: 0.20
Component: mythtv Version: head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

i have one recording that always causes the frontend to segfault.
everytime i start the recording i get 2 lines of text at the top saying:
"P692 Page Not Available"
"TT CC1: Norweigiar"
the first one have a black background and the second line fades away.

if i jump forward or watch for a few minutes the frontend segfaults with the attached backtrace.

i suspect this is somehow related to the dvb teletext subtitles, i have never seen them actualy work before so maybe this is a new thing in svn?

# mythfrontend --version
Library API version: 0.19.20060522-1
Source code version: 10067
Options compiled in:
 linux debug using_xvmcw using_v4l using_oss using_alsa using_arts using_ivtv using_dbox2 using_hdhr using_ip_rec using_lirc using_joystick_menu using_dvb using_x11 using_xv using_xrandr using_xvmc using_xvmc_vld using_frontend using_backend

Attachments (5)

fe-crash.txt.bz2 (7.8 KB) - added by torbjorn.jansson@… 13 years ago.
gdb backtrace
1899-v1.patch (28.7 KB) - added by danielk 13 years ago.
Fix for segfault
1899-v1.patch segfault.txt.bz2 (8.3 KB) - added by torbjorn.jansson@… 13 years ago.
bt from segfault with patch 1899-v1.patch
1899-v2.patch (3.8 KB) - added by pholmes@… 13 years ago.
Allows the Osdtypeteletext to use m_surface (not normally allowable)
1899-v3.patch (35.4 KB) - added by pholmes@… 13 years ago.
Now uses the OSD to request a redraw of the data. This works fine for subtitles. For teletext pages the Header update has been disabled since this causes an excessive load.

Download all attachments as: .zip

Change History (15)

Changed 13 years ago by torbjorn.jansson@…

Attachment: fe-crash.txt.bz2 added

gdb backtrace

comment:1 Changed 13 years ago by danielk

Milestone: 0.20
Owner: changed from Isaac Richards to danielk

comment:2 Changed 13 years ago by pholmes@…

The problem is in the osdtypeteletext.cpp file. This code makes use of the 'm_surface' variable supplied from the OSD class. The problem is that it will access this without setting the OSD locks so when they are both using it the system crashes.

I implemented the following changes to fix this (sorry not sure how to use the diff tool yet).

file: osdtypeteletext.h.

Added #include "osd.h" In class OSDTypeTeletext Modified constructor from

OSDTypeTeletext(const QString &name, TTFFont *font, QRect displayrect, float wmult, float hmult);

to

OSDTypeTeletext(const QString &name, TTFFont *font, QRect displayrect, float wmult, float hmult, OSD *osd);

Added private var

OSD *parent_osd

file: osd.h

In class OSD Added to the public functions

void SurfaceLock?(void) {osdlock.lock();}; void SurfaceUnLock?(void) {osdlock.unlock();};

file: osd.cpp

in function OSD::InitTeletext? modified

OSDTypeTeletext *ttpage = new OSDTypeTeletext(name, font, area, wmult, hmult);

to

OSDTypeTeletext *ttpage = new OSDTypeTeletext(name, font, area, wmult, hmult, this);

file: osdtypeteletext.cpp

Changed:

OSDTypeTeletext::OSDTypeTeletext(const QString &name, TTFFont *font, QRect displayrect, float wmult, float hmult)

to

OSDTypeTeletext::OSDTypeTeletext(const QString &name, TTFFont *font, QRect displayrect, float wmult, float hmult, OSD *osd)

also added

parent_osd(osd) to the initialized variables list

in function void OSDTypeTeletext::PageUpdated?(int page, int subpage) changed

if (m_surface != NULL) {

m_surface->SetChanged?(true); m_surface->ClearUsed?(); DrawPage?();

}

to

if (m_surface != NULL) {

parent_osd->Surface_Lock(); m_surface->SetChanged?(true); m_surface->ClearUsed?(); DrawPage?(); parent_osd->Surface_UnLock();

}

in function void OSDTypeTeletext::HeaderUpdated?(unsigned char *page, int lang) changed

if (m_surface != NULL)

DrawHeader?(page, lang);

to

if (m_surface != NULL) {

parent_osd->Surface_Lock(); DrawHeader?(page, lang); parent_osd->Surface_UnLock();

}

After these changes the wife has not reported any more crashes while watching her shows with the subtitles (teletext page 801) on.

comment:3 Changed 13 years ago by danielk

pholmes, can you make a patch for this?

If you made the changes against SVN head, "svn diff > 1899-v1.patch" in the mythtv directory should do it.

Changed 13 years ago by danielk

Attachment: 1899-v1.patch added

Fix for segfault

comment:4 Changed 13 years ago by danielk

Type: defectpatch

Torbjorn, I've created a patch which gets rid of the m_surface variable. There is one possible problem in that PageUpdated?() and HeaderUpdated?() can no longer call the draw functions. Of cource, they shouldn't be calling draw functions, and the SEGFAULT is just one reason why. But we may need to tell the OSD that it needs to repaint itself...

Please test... I would like to know if it basically works, and whether we need to schedule a repaint (i.e. pages are not updating themselves).

comment:5 Changed 13 years ago by pholmes@…

Sorry for not getting back with a patch. My system is at home so it takes a at least a day to respond normally.

The current change of scheduling a redraw will not work correctly in the case of VBI_IVTV. This is due to the function OSDTypeTeletext::AddPageHeader? that shall call PageUpdated? that will schedule the redraw will also erase the current page data (memset(ttpage->data[0]+40, ' ', sizeof(ttpage->data)-40); - as a side point this should always be done for any VBI type. If not, and the VBI does not always supply all rows of text you will get old subtitles left on the screen) The fix should be just to copy the current page data to a draw page that the draw function us.

  1. Holmes

comment:6 Changed 13 years ago by torbjorn.jansson@…

Daniel, i tested the patch and it causes an immediate segfault when i try to start the bad recording (black screen, then segfault). Without the patch i get atleast a few seconds of video before it crashes.

I've attached a backtrace from the coredump with the patch. i had to kill gdb because it just repeated the last lines forever.

Changed 13 years ago by torbjorn.jansson@…

bt from segfault with patch 1899-v1.patch

Changed 13 years ago by pholmes@…

Attachment: 1899-v2.patch added

Allows the Osdtypeteletext to use m_surface (not normally allowable)

comment:7 Changed 13 years ago by danielk

pholmes, I don't think allowing teletext to use m_surface in this way is the proper solution. But adding some caching/double-buffering as suggested in your comment sounds like a good way to go about this. Just clearing the data is problematic since it can take a while before we get a new screen and we may need to do an OSD redraw in the meantime for some reason unrelated to VBI.

comment:8 Changed 13 years ago by pholmes@…

To danielk

I totally agree. The use of the m_surface is a bad ideal. I checked to see how the other closed caption routines work and all of those have a callback to the osd to schedule a draw request. This callback is normally made via NVP but since the DVB teletext is separate you might still have to provide a osd reference to use. So using the work you have already started and adding double buffering of the data and a callback to request a redraw should work. One other issue that might have to be addressed is the order in which the OSD does a redraw. The last thing you want is for the teletext to be the last item drawn. If it is then all other OSD is hidden (since drawing a blank teletext page actually clears all other data). The current version that make use of the m_surface variable has the effect that if I display the menu and then get a new page of caption it hides the menu.

Changed 13 years ago by pholmes@…

Attachment: 1899-v3.patch added

Now uses the OSD to request a redraw of the data. This works fine for subtitles. For teletext pages the Header update has been disabled since this causes an excessive load.

comment:9 Changed 13 years ago by pholmes@…

The changes done in 1899-v3.patch were as follows: -teletext pages (and subtitles) are now buffered. -When a teletext page being viewed changes an OSD update is requested (direct drawing on the surface has been removed) -Change the OSD priority for the teletext page to 30 to allow other OSD items to be drawn on top of the teletext page.

Todo: -Teletext page headers are no longer being redrawn when a new header is received. Either need the OSD to allow a partial redraw, or add a timer to limit updates to 1 per second.

comment:10 Changed 13 years ago by danielk

Resolution: fixed
Status: newclosed

(In [10355]) Fixes #1899. Fixes segfault, caused by drawing at the wrong time in the teletext menu code.

Peter Holmes found the problem and modified my initial fix so that it actually works (I'm not in teletext land). Basically this makes the teletext display class cache up any data to be drawn and draws it when in the draw method instead of just drawing to the osd surface willy nilly whenever data arrives, and adds some locking to prevent the the data update and the drawing threads from accessing the data at the same time.

This also adds the file to translate.pro, one error string was always shown in English...

Note: See TracTickets for help on using tickets.