Opened 18 years ago
Closed 18 years ago
#1899 closed patch (fixed)
frontend segfault (dvb teletext subtitle related?)
Reported by: | 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)
Change History (15)
Changed 18 years ago by
Attachment: | fe-crash.txt.bz2 added |
---|
comment:1 Changed 18 years ago by
Milestone: | → 0.20 |
---|---|
Owner: | changed from Isaac Richards to danielk |
comment:2 Changed 18 years ago by
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 18 years ago by
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.
comment:4 Changed 18 years ago by
Type: | defect → patch |
---|
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 18 years ago by
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.
- Holmes
comment:6 Changed 18 years ago by
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 18 years ago by
Attachment: | 1899-v1.patch segfault.txt.bz2 added |
---|
bt from segfault with patch 1899-v1.patch
Changed 18 years ago by
Attachment: | 1899-v2.patch added |
---|
Allows the Osdtypeteletext to use m_surface (not normally allowable)
comment:7 Changed 18 years ago by
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 18 years ago by
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 18 years ago by
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 18 years ago by
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 18 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(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...
gdb backtrace