Database updates in firewirerecorder.cpp cause dropped data

[ajlill@…]

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.

[danielk]

use devicereadbuffer for ivtv

[danielk]

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.

[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.

[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.

[danielk]

Moved patch from #712 (a duplicate).

[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.

[ajlill@…]

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

[ylee@…]

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" .)

[wilhelm.eger@…]

Patch for SVN ver. 12546

[wilhelm.eger@…]

Patch for SVN Ver. 12631

[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?

[ajlill@…]

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

[GuyPaddock]

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?

[ajlill@…]

Updated to 12694 and cleaned as requested

[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.

[anonymous]

Isaac, stop lowering the priority of this ticket.

[danielk]

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

[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...

[cpinkham]

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.

[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.

[cpinkham]

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.

[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.

[ylee@…]

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.)

[ajlill@…]

Updated patch with retry logic

[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.

[cpinkham]

Updating description to reflect current status.

[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.

[cpinkham]

(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.