Opened 13 years ago

Closed 11 years ago

#1660 closed defect (fixed)

Database updates in firewirerecorder.cpp cause dropped data

Reported by: ajlill@… Owned by: cpinkham
Priority: minor Milestone: 0.21
Component: mythtv Version: head
Severity: medium Keywords:
Cc: nick.rosier@… Ticket locked: no

Description

I've been fighting the

kernel: ivtv0: All encoder MPEG stream buffers are full. Dropping data. kernel: ivtv0: Cause: the application is not reading fast enough.

problem for months now. It turns out the biggest problem is updating the filesize and recordedmarkup in the same thread that reads from /dev/video. The attached patch creates a separate thread to run theses, redoes the multilple inserts to be more efficient, as well as changes to up the priority of the reader and writer threads. With these changes, the frequency of these errors have dropped from once per program to once per week.

Attachments (9)

speedup.patch (21.2 KB) - added by ajlill@… 13 years ago.
1660-v1.patch (7.1 KB) - added by danielk 13 years ago.
use devicereadbuffer for ivtv
hdtvrec-drb.patch (32.5 KB) - added by danielk 12 years ago.
Moved patch from #712 (a duplicate).
asyncdb.patch (15.0 KB) - added by ajlill@… 12 years ago.
Updated speedup.patch to 0.20 - this is just the changes to make the programinfo class use an asynchronous db thread, since it make the realtime fixes superflous
asyncdb.2.patch (15.0 KB) - added by ylee@… 12 years ago.
Fixed version of ajlill@…'s asyncdb.patch for 0.20 with two typos fixed that caused a "malformed patch" error. (In line 5, both ",11"s should be ",9" .)
asyncdb.12546.patch (14.2 KB) - added by wilhelm.eger@… 12 years ago.
Patch for SVN ver. 12546
asyncdb.12631.patch (14.3 KB) - added by wilhelm.eger@… 12 years ago.
Patch for SVN Ver. 12631
asyncdb.12694.patch (12.8 KB) - added by ajlill@… 12 years ago.
Updated to 12694 and cleaned as requested
asyncdb.13130.patch (13.3 KB) - added by ajlill@… 12 years ago.
Updated patch with retry logic

Download all attachments as: .zip

Change History (34)

Changed 13 years ago by ajlill@…

Attachment: speedup.patch added

comment:1 Changed 13 years ago by danielk

Owner: changed from Isaac Richards to danielk

Changed 13 years ago by danielk

Attachment: 1660-v1.patch added

use devicereadbuffer for ivtv

comment:2 Changed 13 years ago by danielk

Milestone: 0.20
Version: head

ajlill, can you try the attached patch?

It uses the DeviceReadBuffer? we use for the DVB recorder for the ivtv recorder. This should allow us to keep reading the ivtv device while the DB inserts are in progress.

I'm still interested in some of your improvements if you could break them into a patch for each one I might be able to apply at least some of them.

However, please test this patch without those improvements. I think this small change alone should be sufficient to address your problem.

comment:3 Changed 13 years ago by ajlill@…

I can put it in this weekend and run it over next week to see how it works. The only other thing in the patch was monkeying with priorities in the reader and writer threads. If your patch works, I'll have to move the priority bump from the MPeg thread to DeviceReadBuffer?, and I'll submit a new ticket.

comment:4 Changed 13 years ago by ajlill@…

I gave your patch a short trial (backported it to 0.19, actually) and I didn't see any ivtv errors during recording. However, it doesn't look like it stops recording properly. 15 seconds after the recording is supposed to end, I get the ivtv error, probably beause the driver is producing data after the DeviceReadBuffer? thread ends. This causes mythfrontend to crash when you change channels and looks like it skips a noticable ammount of time on consecutive recordings. Because of these problems I couldn't run it very long.

Changed 12 years ago by danielk

Attachment: hdtvrec-drb.patch added

Moved patch from #712 (a duplicate).

comment:5 Changed 12 years ago by kanetse@…

So I've been running the speedup.patch on my system for the past two days, and my ivtv errors have completely disappeared. I was also getting huge numbers of dropped frames every time a recording was just starting (on the order of 5 to 10 seconds), and those frame drops have also disappeared.

It was compiled into release-0.19-fixes.

comment:6 Changed 12 years ago by danielk

Milestone: 0.200.21

Changed 12 years ago by ajlill@…

Attachment: asyncdb.patch added

Updated speedup.patch to 0.20 - this is just the changes to make the programinfo class use an asynchronous db thread, since it make the realtime fixes superflous

Changed 12 years ago by ylee@…

Attachment: asyncdb.2.patch added

Fixed version of ajlill@…'s asyncdb.patch for 0.20 with two typos fixed that caused a "malformed patch" error. (In line 5, both ",11"s should be ",9" .)

comment:7 Changed 12 years ago by anonymous

Cc: nick.rosier@… added

Changed 12 years ago by wilhelm.eger@…

Attachment: asyncdb.12546.patch added

Patch for SVN ver. 12546

Changed 12 years ago by wilhelm.eger@…

Attachment: asyncdb.12631.patch added

Patch for SVN Ver. 12631

comment:8 Changed 12 years ago by cpinkham

The asyncdb.12631.patch version is incomplete, it appears to be missing changes to mythbackend/main.cpp where gAsyncDB is setup. Are there any other missing parts? If the QStringList is working fine, can the other adb_cmd_list parts be cleaned out?

comment:9 Changed 12 years ago by ajlill@…

If you download it in the original format, it's complete. There's something wonky with the patch fancy viewer.

comment:10 Changed 12 years ago by cpinkham

Owner: changed from danielk to cpinkham

comment:11 Changed 12 years ago by GuyPaddock

Priority: minormajor

When, oh when, will this patch make it in to SVN?

This problem has been plaguing all of my recordings, and I've been installing the latest build from SVN every week in the hope that it would disappear. Now that I've discovered this official bug and see that it dates back to April, I have to ask, what's the holdup?

comment:12 Changed 12 years ago by Isaac Richards

Priority: majorminor

Changed 12 years ago by ajlill@…

Attachment: asyncdb.12694.patch added

Updated to 12694 and cleaned as requested

comment:13 Changed 12 years ago by ajlill@…

I cleaned out the adb_cmd_list. The patch by wilhelm.eger@… had a space missing at the beginning of a line that tricked the patch viewer (and the patch command) into thinking a couple of the changes to main.cpp was to another file. Strange... Anyway, this should be good to go.

comment:14 Changed 12 years ago by anonymous

Priority: minormajor

Isaac, stop lowering the priority of this ticket.

comment:15 Changed 12 years ago by danielk

Priority: majorminor

Unless your name is Chris Pinkham, don't mess with the priority.

comment:16 Changed 12 years ago by anonymous

Don't you care when users of your software are encountering a problem that inhibits the function of the program you're writing?

It's only a matter of time before this project gets forked...

comment:17 Changed 12 years ago by cpinkham

Status: newassigned

Can you guys try out my asyncdb_v1.diff version of the patch. It has a couple changes. It now takes a pointer to a MSqlQuery as an argument instead of a QString. This way we can continue using the prepare()/bindValue() methodology rather than having to build QString's all over the place and dealing with having to quote, format dates, etc.. The second change is the addition of a callback and data pointer to the add command. This allows you to specify a callback procedure to be called when the query is executed. The callback procedure will be called with the original MSqlQuery pointer and the passed-in data pointer as arguments. I added this because one of the things on my TODO list for a long time has been to try to optimize the loading of the seektable when we start playback. If we can load the seektable in the background, we can start playback quicker.

I've done minor testing of this on my dev system and everything appears to be working fine. I even tested the callback, but took the test callback out before I made this patch. Can you please try this out and report back and hopefully we can get this into SVN soon.

comment:18 Changed 12 years ago by cpinkham

Nix the whole async idea. After talking this over on IRC, we are going to move these DB updates outside of the recorder and into the main TVRec class. The various *recorder classes will continue to populate the delta map but the TVRec class will be the consumer and will write the data out to the DB. I'm removing the patch I mentioned in my previous comment.

comment:19 Changed 12 years ago by cpinkham

Resolution: fixed
Status: assignedclosed

For now, this should have been fixed in trunk in [12918] and in -fixes in [12919]. These changesets disable updating the filesize inside the main recording loop in order to get rid of the contention between the scheduler and the recorders.

I'm going to close this ticket for now, but may update it again for reference if we make any more changes related to updating the filesize during recording.

comment:20 in reply to:  19 Changed 12 years ago by anonymous

Replying to cpinkham:

For now, this should have been fixed in trunk in [12918] and in -fixes in [12919].

Fantastic! I just wanted to confirm that this change indeed fixed the ivtv messages as well as the frequent blocky/garbled video and out-of-sync audio.

comment:21 Changed 12 years ago by ylee@…

Resolution: fixed
Status: closedreopened

I'd like to report my experiences with the above patches. Bottom line: I hope ajlill's asyncdb patch stays in the code.

I have two FireWire? cable boxes and an ATSC capture card, and routinely simultaneously record from all three sources. I've been using the asyncdb patch (to be accurate, asyncdb.2.patch with the typo correction I submitted) on top of my 0.20-fixes setup for several months now. With it, 60-minute FireWire? HD recordings typically come in at 59:55 to 59:59; i.e., exactly as long as they should, barring startup times.

Since the ATrpms 0.20-fixes package now incorporates r12919 I tried building it without the asyncdb patch. Not so much luck here; the same 60-minute FireWire? recordings came in about 30-250 seconds short, and during replay I saw frequent MPEG errors from dropped frames (slight skips in the video or the image blurring for a moment). (Interestingly, ATSC HD recordings were seemingly unaffected.)

Changed 12 years ago by ajlill@…

Attachment: asyncdb.13130.patch added

Updated patch with retry logic

comment:22 Changed 12 years ago by ajlill@…

I'v updated my patch to add some logic to restart the database connection if it fails. This only affects people using slave backends, since local database connections automagically restart. If you're using the 0.20 branch, apply the patch for that branch, then replace asyncdb.* files with the ones in this patch.

comment:23 Changed 12 years ago by cpinkham

Summary: database updates in *recorder.cpp cause dropped dataDatabase updates in firewirerecorder.cpp cause dropped data

Updating description to reflect current status.

comment:24 Changed 12 years ago by gap7472@…

I'm definitely still seeing this with my setup and I'm NOT USING FIREWIRE. Using the latest SVN, as of yesterday.

Recommend you look into it again; it's fking annoying as hell.

comment:25 Changed 11 years ago by cpinkham

Resolution: fixed
Status: reopenedclosed

(In [13619]) Move the seektable writing code out from the main recorder threads and into the TVRec main thread. Now the recorder threads fill in the positionMap and positionMapDelta and the TVRec class calls RecorderBase::SavePositionMap?() to consume the data.

If it is determined that we should write the delta to the database, a copy of the delta map is made and the positionMapLock is unlocked. This allows us to keep the position maps for as short of time as possible and allows the DB update to happen without impacting the recorders collecting data.

As part of this, I moved the *Recorder::SavePositionMap?() code into RecorderBase? because the only difference between the three was the positionmap type. The type is now stored in a variable set by the three *Recorder classes affected.

Closes #1660.

Note: See TracTickets for help on using tickets.