Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#6138 closed task (fixed)

HDHomerun Multirec Patches

Reported by: cizek@… Owned by: danielk
Priority: minor Milestone: unknown
Component: mythtv Version: head
Severity: medium Keywords: multirec
Cc: Ticket locked: no

Description

Attached are patches which implement multirec for the HDHomerun.

There are three patches:

  • 132-hdhr.20081231.patch updates the hdhomerun libraries
  • 133-release.19703.0116.multirec.bc.2.patch implements multirec
  • 134-release.19703.0116.recstats.cd.1.patch adds recording statistics logging (buffer highwater mark and error counts):
    2009-01-17 10:32:30.503 HDHRRec(31): HDHR Stats: pkts=423845 hwm=33 KB Errors: net=0 trnsprt=124 seq=1 ovflow=0
    2009-01-17 10:32:30.913 DVBRec(21:1): DVB Stats: pkts=8664718 hwm=491 KB Errors: trnsprt=0 seq=0
    2009-01-17 10:32:31.004 TFW: Buffer size = 8064 KB hwm = 219 KB
    

These are all against the 0.21-fixes branch.

BIG FAT WARNING: This has worked for me, but there might be bugs in it which could cause you to lose recordings or screw up your setup. Backup your database and make sure you know what you're doing if you try it.

Attachments (26)

132-hdhr.20081231.patch (243.2 KB) - added by anonymous 10 years ago.
hdhomerun lib update
133-release.19703.0116.multirec.bc.2.patch (81.2 KB) - added by anonymous 10 years ago.
multirec patch
134-release.19703.0116.recstats.cd.1.patch (15.4 KB) - added by anonymous 10 years ago.
recorder stats patch
135-hdhr.skipretune.patch (1.4 KB) - added by anonymous 10 years ago.
Ignore tuning request if we're already tuned to the channel. Was causing glitches during program transitions.
133-hdhr.multirec.3.patch (82.4 KB) - added by anonymous 10 years ago.
Updated multirec patch. Includes skip tuning patch and fix for crash during recorder teardown.
133-hdhr.multirec.6.patch (84.5 KB) - added by anonymous 10 years ago.
134-hdhr.recstats.6.patch (15.4 KB) - added by anonymous 10 years ago.
133-hdhr.multirec.7.patch (85.0 KB) - added by cizek@… 10 years ago.
Fixes backend crash when changing the channel in liveTV
136-hdhr.dvb.1.patch (4.3 KB) - added by cizek@… 10 years ago.
dvb patches from #6274
133-hdhr.multirec.8.trunk.patch (105.4 KB) - added by cizek@… 10 years ago.
Patch against trunk v20018
133-hdhr.multirec.9.patch (84.4 KB) - added by cizek@… 10 years ago.
132-hdhr.20081231.trunk.patch (106.3 KB) - added by anonymous 10 years ago.
133-hdhr.multirec.9.trunk.patch (83.4 KB) - added by anonymous 10 years ago.
134-hdhr.recstats.trunk.7.patch (15.7 KB) - added by anonymous 10 years ago.
133-hdhr.multirec.10.trunk.patch (84.4 KB) - added by cizek@… 10 years ago.
133-hdhr.multirec.11.trunk.patch (85.4 KB) - added by anonymous 10 years ago.
136-hdhr.conversion.trunk.3.patch (8.6 KB) - added by anonymous 10 years ago.
136-hdhr.conversion.fixes.3.patch (9.2 KB) - added by anonymous 10 years ago.
132-hdhr.20081231.trunk.2.patch (107.7 KB) - added by cizek@… 10 years ago.
6138-dbg1.log (5.9 KB) - added by danielk 10 years ago.
log file showing failure to communicate with hdhomerun device
hdhomerun-multirec-v3.patch (97.1 KB) - added by danielk 10 years ago.
Reviewed version of multirec with rtp->udp fallback.
hdhomerun.multirecidchange.4.patch (8.2 KB) - added by cizek@… 10 years ago.
Allow ID-tuner# or IP-tuner# devicename
hdhr.config.ui.1.patch (18.8 KB) - added by cizek@… 10 years ago.
133-hdhr.multirec.fixes.10.patch (86.1 KB) - added by cizek@… 10 years ago.
133-hdhr.multirec.fixes.11.patch (86.8 KB) - added by cizek@… 10 years ago.
Updated 0.21-fixes patch
133-hdhr.multirec.fixes.12.patch (87.1 KB) - added by anonymous 10 years ago.

Download all attachments as: .zip

Change History (65)

Changed 10 years ago by anonymous

Attachment: 132-hdhr.20081231.patch added

hdhomerun lib update

Changed 10 years ago by anonymous

multirec patch

Changed 10 years ago by anonymous

recorder stats patch

comment:1 Changed 10 years ago by danielk

Milestone: unknown0.22
Owner: changed from Isaac Richards to danielk
Status: newassigned

Changed 10 years ago by anonymous

Attachment: 135-hdhr.skipretune.patch added

Ignore tuning request if we're already tuned to the channel. Was causing glitches during program transitions.

Changed 10 years ago by anonymous

Attachment: 133-hdhr.multirec.3.patch added

Updated multirec patch. Includes skip tuning patch and fix for crash during recorder teardown.

Changed 10 years ago by anonymous

Attachment: 133-hdhr.multirec.6.patch added

Changed 10 years ago by anonymous

Attachment: 134-hdhr.recstats.6.patch added

comment:2 Changed 10 years ago by cizek@…

Updated two patches:

  • 133-hdhr.multirec.6.patch
  • 134-hdhr.recstats.6.patch

These change the hdhomerun device identification to use the {deviceid}-{tuner#} standard used in the hdhomerun config tools.

mythtv-setup now uses the hdhomerun libraries to grab a list of available devices and tuners, and lets the user select from this list (similar to the DVB card configuration).

Changed 10 years ago by cizek@…

Attachment: 133-hdhr.multirec.7.patch added

Fixes backend crash when changing the channel in liveTV

comment:3 Changed 10 years ago by cizek@…

I saw a couple of questions on the dev list; I'm answering them here since they relate to this ticket:

The current (Feb 3, 2009) patchset is

  • 132-hdhr.20081231.patch
  • 133-hdhr.multirec.7.patch
  • 134-hdhr.recstats.6.patch

135-hdhr.skipretune.patch is obsolete.

Other points:

  • This doesn't change any database structure.
  • This requires the 20081231 firmware for the HDHomerun.
  • Because the tuner-id format has changed for multirec, you must delete and re-configure your HDhomerun tuner cards. If you roll-back, you must again delete and re-configure the cards.
  • I only have one HDHomerun. This should work with more than one but it isn't tested.

comment:4 Changed 10 years ago by Nigel

(In [20010]) Minor tidyup, add DVB-T compatible modulation selection. See #6274. (Sorry, will break a multirec patch. See #6138)

Changed 10 years ago by cizek@…

Attachment: 136-hdhr.dvb.1.patch added

dvb patches from #6274

comment:5 Changed 10 years ago by cizek@…

Added 136-hdhr.dvb.1.patch (similar to #6274). This should make this patchset work with DVB. My HDHR is ATSC, so I don't know if it actually works for DVB, but it still works with ATSC.

All patches are against 0.21-fixes

The current (Feb 18, 2009) patchset is

  • 132-hdhr.20081231.patch
  • 133-hdhr.multirec.7.patch
  • 134-hdhr.recstats.6.patch
  • 136-hdhr.dvb.1.patch (optional, Useful only for DVB)

Changed 10 years ago by cizek@…

Patch against trunk v20018

comment:6 Changed 10 years ago by cizek@…

Added 133-hdhr.multirec.8.trunk.patch

This is a stand-alone patch against trunk. It should work fine for ATSC.

Changed 10 years ago by cizek@…

Attachment: 133-hdhr.multirec.9.patch added

comment:7 Changed 10 years ago by cizek@…

Added 133-hdhr.multirec.9.patch. This patch is updated to work with fixes after commit [20088].

136-hdhr.dvb.1.patch is now obsolete.

All patches are against 0.21-fixes

The current (Mar 3, 2009) patchset is

  • 132-hdhr.20081231.patch
  • 133-hdhr.multirec.9.patch
  • 134-hdhr.recstats.6.patch

Changed 10 years ago by anonymous

Changed 10 years ago by anonymous

Changed 10 years ago by anonymous

comment:8 Changed 10 years ago by cizek@…

Updated the trunk patches. The trunk patchset now includes

  • 132-hdhr.20081231.trunk.patch
  • 133-hdhr.multirec.9.trunk.patch
  • 134-hdhr.recstats.trunk.7.patch (optional, helps debug problems)

Usage is similar to the fixes patchsets as described elsewhere in this ticket. AFAIK this should work as well as normal trunk for DVB.

comment:9 Changed 10 years ago by anonymous

Is there an ETA for when this will be committed to the the 0.21-fixes branch?

comment:10 in reply to:  9 Changed 10 years ago by RyeBrye@…

Replying to anonymous:

Is there an ETA for when this will be committed to the the 0.21-fixes branch?

I will save a dev the time and answer according to mythTV policy: NEVER.

New features are never committed to the -fixes branch.

comment:11 Changed 10 years ago by anonymous

Understandable!

Any plans on committing to trunk?

comment:12 Changed 10 years ago by danielk

Milestone: 0.22unknown
Status: assignedinfoneeded
Type: patchtask
Version: 0.21-fixeshead

Since this patch-set still requires you to add and remove hdhomerun devices it is not ready for committing, pushing to a future release and changing status type from "patch" to "task" since the existing patch is not ready for code review.

comment:13 Changed 10 years ago by cizek@…

Can you clarify what's wrong with the patch, Daniel?

I've been using it since mid-January and it's been working quite well.

-Bill

Changed 10 years ago by cizek@…

comment:14 Changed 10 years ago by cizek@…

Updated against trunk 20674. The trunk patchset now includes

  • 132-hdhr.20081231.trunk.patch
  • 133-hdhr.multirec.10.trunk.patch
  • 134-hdhr.recstats.trunk.7.patch

comment:15 Changed 10 years ago by danielk

Cizek, I didn't actually get as far as looking at the code.

I noticed this comment of yours from 4 months back: <<<Because the tuner-id format has changed for multirec, you must delete and re-configure your HDhomerun tuner cards. If you roll-back, you must again delete and re-configure the cards.>>> Obviously that is unacceptable.

Best case scenario the tuner-id format is unchanged, so that you specify the IP address or device id + tuner # as before. Worst case scenario, but possibly acceptable, the tuner-id format changes but is auto-magically updated by a dbcheck upgrade query.

comment:16 Changed 10 years ago by cizek@…

Daniel, You raise a valid point about the change in format, but I'm not sure how to best handle it.

In doing this work I tried to mirror the DVB multirec logic (with the Stream Handler and associated logic). This requires a single key value to identify an HDhomerun tuner, but such a key is not available in the existing trunk configuration which has two separate fields. I started out by combining the two fields on the fly, and eventually changed the logic to use the deviceid-tuner# format used in the HDhomerun config tools. I don't think there's an easy way to change this in my patch.

IMHO the "Worst case scenario" you mentioned (converting the format with a dbcheck upgrade query) is the easiest way to handle this. I'm busy now but I'll take a look at it when I get some free time.

-Bill

comment:17 Changed 10 years ago by zavex@…

In 133-hdhr.multirec.10.trunk.patch, qdeepcopy.h is included in hdhrstreamhandler.cpp. I believe this file no longer exists in qt4, and the file compiles just fine with out it (but errors out with it).

Changed 10 years ago by anonymous

Changed 10 years ago by anonymous

Changed 10 years ago by anonymous

comment:18 Changed 10 years ago by cizek@…

Added 133-hdhr.multirec.11.trunk.patch to fix the qdeepcopy.h inclusion.

Added 136-hdhr.conversion.trunk.3.patch and 136-hdhr.conversion.fixes.3.patch. These bump DBSchemaVer and convert any existing HDHR configurations to the new format as necessary. Any HDHR lines found with the new format are left as is.

The trunk patchset is

  • 132-hdhr.20081231.trunk.patch
  • 133-hdhr.multirec.11.trunk.patch
  • 134-hdhr.recstats.trunk.7.patch
  • 136-hdhr.conversion.trunk.3.patch

The fixes patchset is

  • 132-hdhr.20081231.patch
  • 133-hdhr.multirec.9.patch
  • 134-hdhr.recstats.6.patch
  • 136-hdhr.conversion.fixes.3.patch

comment:19 Changed 10 years ago by mikeb@…

The patch has a few bugs which seem to be inherited from the original mythtv code.

  • Whenever possible, use the hdhomerun_device helper functions rather than attempting to parse the result from the hdhomerun directly. The current code fails to parse the signal strength correctly if a dBmV value is returned (TECH versions).
  • Avoid calling hdhomerun_control or hdhomerun_video; access to these functions should be done through the similarly named hdhomerun_device calls; the main advantage is this will allow the backend to be started before the network and the control socket reconnected as needed.

The HDHRStreamHandler::Open(), FindDevice(), Connect() sequence is just silly; once you have a hdhomerun_device object you're done.

comment:20 Changed 10 years ago by cizek@…

Thanks for taking a look at this, Mike. Props on the hdhomerun - I've been using it for nearly two years and it's really slick.

I'm not too familiar with the hdhomerun_device calls. I looked into using them when I started the multirec work but gave up when I realized it would turn one big change - multirec - into two big changes - multirec and hdhomerun interface. I ended up focusing on the multirec only and kept the existing interface code as similar as possible to avoid screwing things up. If you notice any regressions in this patch let me know and I'll look into them, but I'd prefer to keep the scope of this work limited to multirec.

That said, when I worked on this I noticed some things that could be improved in the hdhomerun interface in MythTV:

  • Change to use the hdhomerun_device calls you mentioned.
  • Use the tuner locking capability.
  • Add more error handling (I may be wrong, but if communications is lost during a recording, MythTV doesn't notice and won't mark the recording as Aborted so it can be rescheduled)
  • Combine the DVB and hdhomerun stream handling logic. While there are specific ways to tune and filter data from the different device types, it seems to me that the data stream and it's handling should be the same between the two.

-Bill

comment:21 Changed 10 years ago by mikeb@…

If the multirec patches were ever merged (hint hint) then a new set of patches could be done to clean up the hdhomerun handling but it seems counterproductive to start new patches based on the non-multirec code.

Changing from the hdhomerun_control to hdhomerun_device is actually pretty trivial, just rename the calls from hdhomerun_control_* to hdhomerun_device_* and change the first argument from _control_sock to an hdhomerun_device object.

comment:22 Changed 10 years ago by anonymous

Patch 136-hdhr.conversion.fixes.3.patch does not apply to latest release-0-21-fixes branch.

It incorrectly assumes

currentDatabaseVersion = "1215"

when in fact is is

currentDatabaseVersion = "1214"

comment:23 in reply to:  22 Changed 10 years ago by cizek@…

Replying to anonymous:

Patch 136-hdhr.conversion.fixes.3.patch does not apply to latest release-0-21-fixes branch.

It incorrectly assumes

currentDatabaseVersion = "1215"

when in fact is is

currentDatabaseVersion = "1214"

You are correct... I accidentally made the patch on top of an unrelated change.

After I posted the 136-hdhr.conversion patches I realized that they really shouldn't be used in a patched system. The reason being that they update the DBSchemaVer value and can interfere with future database schema upgrades (especially when going from fixes to trunk). These are geared more towards the devs and eventual inclusion of this functionality into trunk. Once this patchset is merged into trunk, the DBSchemaVer bump will be part of the official release and cause no problems.

Bottom line. If you know what you're doing and you just want to get hdhr multirec on a fixes or trunk system, use the first three patches (132, 133, 134) and skip the 136-hdhr.conversion patch, and then delete and reconfigure your HDHR tuners. Once everything's merged into trunk the delete / reconfigure step won't be necessary.

Sorry about the confusion. I hope this didn't screw anything up for anyone.

-Bill

comment:24 Changed 10 years ago by danielk

cizek, 2 problems.

1/ The 132 patch appears to reintroduce old bugs. For instance, replacing:

  • /* can't use memset bcs sequence is volatile */
  • for (int i = 0; i < sizeof(vs->sequence) / sizeof(uint8_t) ; i++)
  • vs->sequence[i] = 0xFF;

with + memset((void *)vs->sequence, 0xFF, sizeof(vs->sequence));

2/ Where is the UI patch? i.e. the patch that allows me to specify the number of virtual hdhomerun recorders to create for each real one, as with the DVB recorders, via mythtv-setup?

Changed 10 years ago by cizek@…

comment:25 Changed 10 years ago by cizek@…

Added 132-hdhr.20081231.trunk.2.patch. This includes the changes from [19496] and [20451]. Hopefully it addresses the bugs you mentioned, Daniel. When I made the patch I simply overlaid the myth source with the new hdhomerun library source files so these changes got lost.

The UI changes are all included within the 133 patch. Similar to the DVB card setup, the user must press the "Recording Options" button to open a new screen to select the number of virtual tuners to use. Unfortunately this button spans the whole screen width and is easy to miss. I'd like to change it (either shrink the button or get rid of the whole second screen altogether) but I'm weak on the UI side and have had no luck with it. If you have any suggestions or if anyone else can help clean up that screen I'd appreciate it.

-Bill

comment:26 Changed 10 years ago by danielk

Bill, thanks I found the Recording Options button, don't worry I can fix that..

I'm now done reading through the code and I'm testing and I've run into a few problems. The first is that it appears not to work, I believe this is the important error, but I'll attach some logs too:

Error: DeviceSet?(/tuner1/target rtp://192.168.1.141:60149): ERROR: malformed or unsupported target mode

The second is that the UI (and maybe the code too?) does not allow you to specify an IP for the device for when discovery doesn't work (due to it being blocked by the router).

The third is that the conversion routine depends on discovery and takes a long time to run.. it should simply take the IP address or ID of the HDHomeRun and append "-" and the tuner number.

FYI For the 132 patch I expect whoever does the merge, to do a three way merge using the common ancestor. The point is to check the changes and make sure they all make sense. But we can skip this for now, it's not really part of multirec.

Changed 10 years ago by danielk

Attachment: 6138-dbg1.log added

log file showing failure to communicate with hdhomerun device

comment:27 Changed 10 years ago by cizek@…

Daniel, my first thought on the DeviceSet? error is that you may have old firmware which doesn't support rtp. A quick fix / test is to change "rtp:" to "udp:" in the 133 patch and see if that helps (It's a one-line change). FWIW I'm using the 20081231 firmware. In any case I'll fix it to fallback to udp if rtp is not available.

I'll have to think about the device discovery items you brought up. I was hoping to make the configuration easier but you make valid points about being able to punch in the IP or the ID if discovery takes too long or doesn't work.

-Bill

comment:28 Changed 10 years ago by danielk

Cizek, rtp->udp did it, and yes it is old firmware. It's been upgraded a couple times, but I stopped upgrading the firmware when the it started working well.. No one should have to upgrade the firmware on our account.

I understand about making configuration easier, but the IP based setup option was added to handle real problems with those little wifi + 4 port routers that a lot of people have at home, I don't want to lose that functionality. It's ok to force people to just input the IP address without any sanity checking in that case, we've already lost auto-magic when discovery fails to see the device, so this will be one of many steps in their setup (they will already need to assign a static DHCP address to the device).

Changed 10 years ago by danielk

Attachment: hdhomerun-multirec-v3.patch added

Reviewed version of multirec with rtp->udp fallback.

comment:29 Changed 10 years ago by danielk

The patch I attached is almost ready to go. The only problem remaining is the configuration. Please apply any additional patches on top of the patch I just generated, you can use quilt to do it. That will allow me to review the additional code more quickly.

comment:30 Changed 10 years ago by cizek@…

For reference, Daniel:

I made some changes to allow for either device_id-tuner# or device_ip-tuner# videodevice naming in the HDHRStreamHandler. This should allow for easier configuration for the "Must use IP address" case.

My changes only affect the stream handler, so they don't address the configuration screen aspect of the problem, but they should help on the backend.

I'll post them when I've merged them against your latest patch.

Changed 10 years ago by cizek@…

Allow ID-tuner# or IP-tuner# devicename

Changed 10 years ago by cizek@…

Attachment: hdhr.config.ui.1.patch added

comment:31 Changed 10 years ago by cizek@…

Added hdhr.config.ui.1.patch

This is preliminary UI functionality to allow configuration by either automatically discovered hdhomerun deviceid or manually entered IP address. It should work but it needs additional error checking to ensure the configuration is valid.

comment:32 Changed 10 years ago by danielk

Resolution: fixed
Status: infoneededclosed

(In [20771]) Fixes #6138. Implements multirec for HDHomeRun based recorders.

Changed 10 years ago by cizek@…

comment:33 Changed 10 years ago by cizek@…

Added 133-hdhr.multirec.fixes.10.patch. This works with the fixes branch after the hdhomerun update (#6672 and [20877])

For simplicity, this patch ignores the hdhomerun interface changes Daniel made in [20877], but if you were successfully using 133-hdhr.multirec.9.patch the new patch should work the same.

132-hdhr.20081231.patch is no longer necessary.

-Bill

comment:34 Changed 10 years ago by ylee@…

Comment to get on cc: list

Changed 10 years ago by cizek@…

Updated 0.21-fixes patch

comment:35 Changed 10 years ago by cizek@…

Added 133-hdhr.multirec.fixes.11.patch

This applies to the current -fixes branch. It should work the same as the 133-hdhr.multirec.fixes.10.patch but is untested.

comment:36 Changed 10 years ago by mikeb@…

I'd suggest taking a look at the current HDHomeRun code in trunk as it appears you've reverted a number of patches.

Changed 10 years ago by anonymous

comment:37 Changed 10 years ago by cizek@…

Added 133-hdhr.multirec.fixes.12.patch

This re-adds the functionality from [21390] into this patchset.

comment:38 Changed 10 years ago by anonymous

I'm afraid I've gotten lost in this. Is there a version of the multirec hdhomerun patch that cleanly builds against the current svn?

Will it ever be integrated?

comment:39 Changed 10 years ago by robertm

Current trunk already supports HDHR multirec. No .21 patch for multirec will ever be applied.

Note: See TracTickets for help on using tickets.