Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#13114 closed Patch - Bug Fix (fixed)

Import Recorder Fix

Reported by: devmonkey@… Owned by: Peter Bennett
Priority: minor Milestone: 30.0
Component: MythTV - Recording Version: 0.28.1
Severity: medium Keywords:
Cc: Ticket locked: no

Description

I've created a pull request for a fix for the Import Recorder for the following issues. Should also cover https://code.mythtv.org/trac/ticket/11117

  • Recordings always marked failed causing lots of rescheduled recordings and broken duplication detection etc
  • Seek table not created, ringbuffer file not marked for write causing the comm flag player to exit early rather than wait for the file to finish writing
  • Recording duration not updated, causes additional seek issues when trying to play an in progress recording

Attachments (3)

mythtv_import_evtest.py (2.9 KB) - added by devmonkey@… 2 years ago.
Import recorder event test script
mythtv_import.sh (224 bytes) - added by devmonkey@… 2 years ago.
realtime file import test script
import_rec_start.sh (1.7 KB) - added by Peter Bennett 2 years ago.
Peter's version of recording start script

Download all attachments as: .zip

Change History (24)

comment:1 Changed 3 years ago by paulh

Reporter: changed from nodenet@… to devmonkey@…

comment:2 Changed 3 years ago by Peter Bennett

comment:3 Changed 3 years ago by Stuart Auchterlonie

Milestone: needs_triage30.0
Owner: changed from JYA to Peter Bennett
Status: newassigned
Version: Unspecified0.28.1

comment:4 Changed 3 years ago by Peter Bennett

Type: Bug Report - GeneralPatch - Bug Fix

comment:5 Changed 3 years ago by Peter Bennett

Status: assignedinfoneeded

Please explain to me how you use "Import Test Recorder" for real recordings. It is mentioned in the wiki that this can be done with certain user jobs, but it is not clear to me how it works. For example, what do you enter for filename when creating the import recorder, or do you leave it blank? Do you use a program guide to schedule recordings? What do you do in the various jobs and how does the recording data get into MythTV? Does the import test recorder work for LiveTV?

I would like to know all this so I can test and verify the fix, also to document in the wiki so that it is clear to all how to do this.

Is this used if you have some type of input not supported by mythtv but supported by some other application? Is it used if you have a program recorded elsewhere and want it in MythTV?

Is the import used realtime while the recording is happening and with the scheduler, or after recording some other way to inject programs into MythTV?

comment:6 Changed 3 years ago by devmonkey@…

Hi, thanks for reviewing this. You may get a bit more insight about what I'm using this for from https://forum.mythtv.org/viewtopic.php?f=9&t=2243

To try and answer your questions

According to the wiki and setup I believe you should leave the filename blank. This does however bring the backend down. In the end as a workaround I just put in a dummy name of a file that does not exist (it will create a link otherwise). I have only a vague recollection of the crash but I did do a backtrace and believe it is crashing when the scheduler runs before or during FindConflict? (or something similar). I believe there is a bad pointer being fed through in the code. It could also be that my database is old and has something missing although I did create the Import recorder from scratch. At the moment this bug is a mute point without the above patch as the Import recorder doesn't do anything useful (I can file a bug if you're happy to merge the other stuff). As a side note I also tried the Demo recorder which fails for different reasons (I believe there is a memory leak at some point).

Program guide is important for scheduling the recordings. They are scheduled on the STB via a script that syncs between mythtv and the STB. Playback is matched up again using an ID trapped during scheduling against channel & start time. In short I'm using playback on the STB rather than tuning a channel (although this can be nearly live as per most DVRs).

In regards how data gets into MythTV with the patch the operation should nominally be as per usual i.e. data is from the program guide and MythTV updates everything accordingly (recording always assumed to have succeeded). The recording itself is generated by writing to the ringbuffer file with a modified version of the hdpvr2 app from the hauppauge web site. I would have some choice words about the hdpvr2 but with a lot of effort you can make it work pretty well. You can otherwise probably get acceptable results fixing your STB to 720p. I think then you may get away with the hdpvr2 app as is or with some minimal mods. Hopefully there are some other devices that could be used though.

I personally never use LiveTV so haven't even tested it. I did note that the channel changer script is disabled so unless the recorder restarts for each channel change I think it's probably a no. I suspect even with the channel changer script enabled that there may be some work required. I also altered the timeout on opening the file to reduce the logging but it could be changed back perhaps with another mod to give the logging a different timeout period.

My goal was to have nominal operation as close to an internal card as possible (I have dual DVB-S2 for other recordings). Once you schedule a recording my backend obviously behaves as normal i.e. in my case it wakes up and waits for the recording to finish. It therefore seemed like a good idea to follow this through and make MythTV import whilst the STB is recording. With the above patch you can also watch while it records as per normal. This only works during the scheduled recording time as the frontend needs to understand that the file is still being updated (default frontend logic or whatever). Obviously when there are 2 simultaneous recordings on the STB things will get out of sync as we can only import one at a time so the second recording will be imported late even if they are back to back. This will kill or mess up the live recording functionality. I think a workaround might be to set a new start / stop time when the import starts.

I do now have static import scripts that don't need anything to be scheduled by MythTV and these don't rely on the Import recorder at all. This just generates a bunch of database entries after doing some lookups etc. It seems to make most sense though to have MythTV do the scheduling in which case it wants to record during the scheduled time etc. I did originally think about doing a daily import of all unwatched programs on the STB but it is less efficient, there is less relevant feedback and the ability to watch an in progress recording isn't there.

One thing which is a problem at the moment is that the full path of the ringbuffer file is not fed to the script parameters. This means none of the live stuff works if you write to a different path than the one MythTV selected. I think maybe someone filed a bug or posted about this somewhere (sorry I don't remember where). I use the Python bindings atm to poll the recorder until the correct file comes up so I can get the full path.

Ultimately I can use MythTV scheduling & other functionality and I don't have to manage 2 systems plus I don't have to put up with the tiny HD & other issues with the STB etc.

comment:7 Changed 3 years ago by devmonkey@…

Forgot to mention I am using MythTV System Events specifically the Recording started event. This is essentially what the wiki and setup program suggest with regards the Import recorder.

comment:8 Changed 3 years ago by Peter Bennett

The wiki recommends putting in a blank filename, but if you did that I don't know how the recorder would know what file to read, so that seems a bogus feature that should not be allowed.

The strange behavior of creating a link if you specify a file that does exist - That does not make sense to me. It should work by copying the file while recording, same as if the file did not exist while setting up the recorder.

I can work with you to get this into the code base. We should also update the wiki with instructions on how to do it. One thing that I wonder about is what happens when you end one recording and start the next, if it starts right away. The first file would have to be deleted and then the next one created with the same file name, before the recorder starts recording.

Regarding your pull request -

  • You have changed the progress log from info to debug. I am not happy with that. Turning on debug gets you huge amounts of data. I would prefer that if you want to change it, make it info level on another category than general. I don't know if there is a suitable category that makes sense.
  • You changed a header file to add virtual to a bunch of methods that are already inheriting virtual. We should not make unnecessary changes.

Have you looked at the "black box" recorder? Would it help with your recording problem or are there other bugs with that?

You mention 2 simultaneous recordings. I assume that if you had 2 STB's and 2 HDMI recorders you could create 2 import recorders with different file names and make it work.

If you fix the things I mentioned above, I will give it a try, although I do not have a suitable recorder to import from but I can try a file. It would be nice if you also have a fix to prevent the creation of links and / or the backend crash when there is no file name. Maybe there should be a parameter somewhere to enable creation of links if people want them.

comment:9 Changed 2 years ago by devmonkey@…

I gather the original idea of the recorder was to create links based on the filename you supply purely for the purpose of testing. I believe the suggestion in the wiki and setup program is to leave the filename blank and use system events to write directly to the recording file if you wish to genuinely import something (this is how I use it). Supplying a fixed filename to import only makes sense in the context of testing as I don't see that being particularly workable otherwise. If the import file in the settings does not exist this will suppress the link generation. So I believe that already supplies a valid method to toggle between testing (link creation) and normal operation (direct write via system event). I believe the demo recorder would be the one to use to make a real-time copy of a file. Presumably the import recorder was created for space / time saving to help test the other functionality of MythTV. I think leaving the link creation in is ok unless we're sure no one does any testing like that any more.

I moved the log message to VB_COMMFLAG, VB_INFO. For my 10 p I would only ever want to see those messages for debugging in the context of other messages when the process was freezing at some point.

virtual has been removed from header that shouldn't have been in there as it was just for readability for me. I believe the override keyword is now preferred anyway.

I did note the black box recorder was there but I haven't tested it. It looks like it would have been more work to essentially do the same thing. Also I'm not sure whether I could make it import anything outside the scheduled recording time so probably another issue to solve that doesn't exist right now. With regards the hdpvr2 app I did do some testing with pipes and named pipes and this was prone to problems with buffer overruns so again more issues to fix that don't exist with a direct file write.

If you were willing to go to the extra cost you could make it work with 2 STBs and 2 recorder devices. Unless I was serving up content for a lot of people I'm not sure I'd need dual import though as sequential import of dual recordings is more cost effective and already working + you could turn that into dual sequential if you really did need that kind of extra capacity. I also suspect the hdpvr2 app would need fixing again to work with 2 recorder devices.

Anyway hopefully that is enough to get this merged as it's currently a broken feature so low risk from the point of regressions. It's also potentially a flexible extension for supporting any new devices where it might not be practical to offer direct support in MythTV.

comment:10 Changed 2 years ago by Peter Bennett

Status: infoneededassigned

comment:11 Changed 2 years ago by Peter Bennett

Status: assignedaccepted

comment:12 Changed 2 years ago by Peter Bennett

Please give me an example of how you are using it (which events do you use and what do the scripts contain). I would like to give it a test and rather than stumble around trying to figure out what works I can look at your scripts. If the scripts are long you can use pastebin. Also what do you set in setup for the file name for the import recorder? It seems impossible to set a file that does not exist, so what do you do? I tried renaming the file after setting it to an existing file, then tried to make a recording, without much success.

Changed 2 years ago by devmonkey@…

Attachment: mythtv_import_evtest.py added

Import recorder event test script

Changed 2 years ago by devmonkey@…

Attachment: mythtv_import.sh added

realtime file import test script

comment:13 Changed 2 years ago by devmonkey@…

I've created a test event script that can also serve as the starting point for a custom import script. There is also a 2nd script 'mythtv_import.sh' that has samples for both a realtime import and link creation script as potential replacements for the existing functionality in the dummy test recorder and import test recorder. This script is an analogy for the script or app that interfaces with the actual recorder hardware (I'm sure you could also use one of the zap programs to capture from one of your existing tuners as well). You will need to fill in a couple of blanks to use these scripts such as file paths etc. The event script contains a sample of the parameters that I use so you should be able to copy them into the recording started system event via the setup program.

I'm not sure why you are not able to a non existent filename are you using v28 or trying this in master. Just wondering if a validation check could have been introduced (there may be a warning but you can ignore it). If there is a check then at least for testing right now you can bypass this by updating the database directly. The filename is stored as the videodevice of the card in the capturecard table. I would propose to do it this way as if you are happy that the sample scripts could be used to replace the existing functionality of link generation and the dummy recorder then I could look at removing that functionality in master and adding a message to refer to the wiki instead for updated instructions.

Please let me know if any extra details are required.

comment:14 Changed 2 years ago by Peter Bennett

Setup - With latest master (Pre-30), and on v29, the import recorder setup takes you to a file browser to select the import file. You cannot select a file that does not exist. You can type in a file name that does not exist and as soon as you move out of the input field, the file name you typed is cleared out and replaced by "/". However, that is all good, because with the file name as "/" it is a file that does not exist and it works as hoped.

I created by own shell script to record from DVB using zap as you suggested. Without your patch, the recording never gets going. With your patch it is recording successfully, just like a normal input card. Attached is my script that has to be used in the recording started event.

I found that using the recording ended event does not work. I had hoped to use a script there to end the recording when MythTV decided the recording was ended. However MythTV did not sent the end recording event until I first killed the ffmpeg, even if it was several minutes past the recording end time. So I switched to timing and killing the recording from inside the start recording event and adding "&" to the start recording event invocation so that it continues running through the entire recording.

I did not fully understand your python script. Do you have a check in there for the card id? I added that in my script because you do not want the import to start up when recording from other types of cards on the system, for example I have a ceton tuner as well, which is recorded in the normal way.

Live TV is a problem. It starts recording perfectly on the backend, but the frontend freezes forever. I would like to have that working as well. It should work, Live TV is perfectly capable of handling tuners that are slow to tune. It may be that the frontend expects always to find some data in the file, if you tune a channel you normally immediately get a file of some 500 bytes, not 0 bytes as we see when starting with import recorder. Note Live TV does work with import recorder if you just use a file copy in the import script so that the file is there by the time the frontend looks for it.

Changed 2 years ago by Peter Bennett

Attachment: import_rec_start.sh added

Peter's version of recording start script

comment:15 Changed 2 years ago by devmonkey@…

Thanks for testing this.

With regards the Python script it was based on my original script but reworked to more or less a general template. To make it work you need to update it with a suitable log path and app path (see comments in the file). The invocation for the RecordingStarted? event would then typically be as follows (details should also be in the comments at the top of the script)

mythtv_import_evtest.py -s %STARTTIMEUTC% -e %ENDTIMEUTC% -f %FILE% -c %CHANID% -t %CARDID%

With regards card selection this is also handled in the event script based on the -t parameter. Afaik there is nothing in the Python bindings that gives the required details on the card. I have therefore added a lookup into the capturecard table using the value parsed using the -t parameter to get the recorder type i.e

SELECT cardtype from capturecard WHERE cardid=%s

The following check then ensures that the card is an import recorder and not some other type of card

if (ct=="IMPORT"):

The duration of the recording is then calculated using the values passed to the -s and -e parameters.

sd=(et - st).total_seconds()

An external script is then backgrounded and detached as follows

(mythtv_import.sh "%s" %d &)

At the moment I have just passed over the file path and schedule duration to this script so you would need to handle the channel selection directly in the Python code or also pass the value from -c to the script or maybe some combination of both (there is an example of how to get the xmltvid for the channel in the Python script if that helps).

A note on the file path as this currently has to be obtained by polling the relevant recorder using the Python bindings. The reason for this is that the event system does not pass over the full path for the recording and the %DIR% variable is actually linked to the playback path so can not be used as it is not available till after the recording file is created. I believe that the backend will make a selection for the recording file path using the storage group rules, this path is then passed internally to the recorder (i.e. not available via the RecordingStarted? event). The import recorder then tries to open this file via the full path so if we start writing the file to a different location than the one the import recorder is expecting it will not work correctly.

It does appear that the backend may block the RecordingFinished? event until the recording is complete (presumably because the ringbuffer is still being indexed and hence the recorder is still running).

I believe this would be a suitable starting point for most people to create an import routine of some sort.

comment:16 Changed 2 years ago by devmonkey@…

With regards Live TV I'm afraid that is pretty much as expected and would class it as a known issue. My guess is that a signal monitor is required to make this work properly. I suspect this would trigger the appropriate messages to the frontend and update the recorder & or recording from tuning to recording status. I think there could also be some work with regards the order and timing of events between the backend and the recorder. I'm not sure I'd have time to look at this right now but maybe in the future. Hopefully this will not be an issue as there is still some benefit to the patch without the full set of functionality i.e. scheduled recordings should work normally.

comment:17 Changed 2 years ago by Peter Bennett

I would like to merge this into MythTV but I do not have an author name and email address other than fullTalgoRythm, and a few strange email addresses.

comment:18 Changed 2 years ago by devmonkey@…

I got a bounce when I tried to email you, please feel free to use the FullTalgoRhythm? details from git

comment:19 Changed 2 years ago by Peter Bennett

fullTalgoRythm does not have any information on your name and the commits do not have a valid email address.

You can email me at pbennett at mythtv.org or pgbennett at comcast.net or peterbenn1 at gmail.com or pb.mythtv at gmail.com -- hopefully one of those will get through.

Last edited 2 years ago by Peter Bennett (previous) (diff)

comment:20 Changed 2 years ago by fullTalgoRythm <devmonkey@…>

Resolution: fixed
Status: acceptedclosed

In c1c127cc2da3a9c194fbfcc4813acfa0be7811b3/mythtv:

Import Recorder: Fix multiple problems

Fix these problems with the Import Recorder

  • Recordings always marked failed causing lots of rescheduled recordings and broken duplication detection etc
  • Seek table not created, ringbuffer file not marked for write causing the comm flag player to exit early rather than wait for the file to finish writing
  • Recording duration not updated, causes additional seek issues when trying to play an in progress recording

With these changes, Import recorder can be viable as a recorder for
recording devices that MythTV does not support directly.

Fixes #13114
Fixes #11117

Signed-off-by: Peter Bennett <pbennett@…>

comment:21 Changed 2 years ago by Peter Bennett

Owner: changed from Peter Bennett to Peter Bennett
Note: See TracTickets for help on using tickets.