MythTV / mythtv

The official MythTV repository
https://www.mythtv.org
GNU General Public License v2.0
708 stars 346 forks source link

Mythexternrecorder requires configuration filename to be fully qualified while mythtv-setup does not #670

Closed JanCeuleers closed 1 year ago

JanCeuleers commented 1 year ago

What steps will reproduce the bug?

I have created an external recorder entry using mythtv-setup, specifying "/usr/bin/mythexternrecorder --conf mythexternrecorder1.conf" as the video device. I then scanned for channels which was successful, suggesting that mythtv-setup was able to find the configuration file as that is where the channels file is specified. (Or perhaps it queried the channels through mythexternrecorder, which means that mythexternrecorder itself was able to find the configuration file and the channels= directive it contains).

However, when actually trying to record something through this external recorder mythexternrecorder cannot find its configuration file, leading to very obscure error messages in the log. I was able to fix that as follows:

mysql> update capturecard set videodevice="/usr/bin/mythexternrecorder --conf /home/mythtv/mythexternrecorder1.conf" where videodevice="/usr/bin/mythexternrecorder --conf mythexternrecorder1.conf";

In other words: mythexternrecorder can only find its configuration file when streaming if it is fully-qualified. Suggesting that the current directory from which mythexternrecorder is invoked differs between mythbackend and mythtv-setup (which are both run as the same user: mythtv).

How often does it reproduce? Is there a required condition?

I only tried this once.

What is the expected behaviour?

  1. If mythexternrecorder cannot open its configuration file it should emit a clear error message to that effect. At the moment this can only be inferred from obscure log entries such as "ERR:Already dead" and "ERR:Not open".
  2. Consistent behaviour between mythtv-setup and mythbackend.

What do you see instead?

As described above: when setting up the external recorder in mythtv-setup one has the impression that all is well because at that time the channel scan works which means that the configuration file was found. Yet when attempting a recording using the external recorder mythexternrecorder cannot find its configuration file, leading to the m_fatal flag being noticed set in MythExternRecApp::Open when called from MythExternRecApp::LockTimeout.

Additional information

jpoet commented 1 year ago

The "External (blackbox) Recorder" mechanism has no way to know what any external recorder application does or does not need for arguments. This means there is no way for it to validate those arguments.

The Wiki page for the "generic" mythexternrecorder application says:

The path to the configuration file must be fully qualified such that mythexternrecorder can find its configuration file regardless of the current directory when it is invoked.

Further, when running mythexternrecorder --help it says:

--conf Path to a configuration file in INI format.

If you would like to update the wiki to make the warning stronger, please do.

JanCeuleers commented 1 year ago

On 27/11/2022 18:22, John Poet wrote:

The "External (blackbox) Recorder" mechanism has no way to know what any external recorder application does or does not need for arguments. This means there is no way for it to validate those arguments.

The Wiki page https://www.mythtv.org/wiki/ExternalRecorder#Using_the_.22Generic.22_External_Recorder for the "generic" mythexternrecorder application says:

The path to the configuration file must be fully qualified such
that mythexternrecorder can find its configuration file regardless
of the current directory when it is invoked.

Further, when running |mythexternrecorder --help| it says:

--conf */Path/* to a configuration file in INI format.

If you would like to update the wiki to make the warning stronger, please do.

— Reply to this email directly, view it on GitHub https://github.com/MythTV/mythtv/issues/670#issuecomment-1328299092, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2CDD77LJBL5UCBK6EVEELWKOKD7ANCNFSM6AAAAAASMPEETE. You are receiving this because you authored the thread.Message ID: @.***>

I already have.

JanCeuleers commented 1 year ago

On 27/11/2022 18:27, Jan Ceuleers wrote:

On 27/11/2022 18:22, John Poet wrote:

The "External (blackbox) Recorder" mechanism has no way to know what any external recorder application does or does not need for arguments. This means there is no way for it to validate those arguments.

The Wiki page https://www.mythtv.org/wiki/ExternalRecorder#Using_the_.22Generic.22_External_Recorder for the "generic" mythexternrecorder application says:

The path to the configuration file must be fully qualified such
that mythexternrecorder can find its configuration file
regardless of the current directory when it is invoked.

Further, when running |mythexternrecorder --help| it says:

--conf */Path/* to a configuration file in INI format.

If you would like to update the wiki to make the warning stronger, please do.

— Reply to this email directly, view it on GitHub https://github.com/MythTV/mythtv/issues/670#issuecomment-1328299092, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2CDD77LJBL5UCBK6EVEELWKOKD7ANCNFSM6AAAAAASMPEETE. You are receiving this because you authored the thread.Message ID: @.***>

I already have.

Having said that, I do consider the lack of clear error message to be a bug. That is: when mythexternrecorder is unable to open the file specified by means of the --conf parameter I submit that this is an error that it can make more of an effort to signal to the user.

jpoet commented 1 year ago

There should have been a message to that effect in the mythexternrecorder log file. Was there not?

JanCeuleers commented 1 year ago

On 27/11/2022 18:36, John Poet wrote:

There should have been a message to that affect in the mythexternrecorder log file. Was there not?

I have no such file. Should it be in /var/log/mythtv/ ?

@.***:~# ls /var/log/mythtv/ mythbackend.log       mythfrontend.log        mythpreviewgen.log mythcommflag.log      mythfrontend.log.1      mythtv-setup.log mythfilldatabase.log  mythmetadatalookup.log

Regardless: would you accept a patch that sends a status message to the backend informing it of the problem, such that an error message (also) appears in the backend log?

(I won't be able to test such a patch, because I have not yet succeeded in building MythTV from source, but I'm hoping that the patch will be simple enough to pass muster).

Thanks, Jan

JanCeuleers commented 1 year ago

On 28/11/2022 16:15, Jan Ceuleers wrote:

On 27/11/2022 18:36, John Poet wrote:

There should have been a message to that affect in the mythexternrecorder log file. Was there not?

I have no such file. Should it be in /var/log/mythtv/ ?

Found it: the mythexternrecorder logging ends up in syslog, and I had missed that. Still: the log message is

Nov 26 13:58:07 hobbiton mythexternrecorder: mythexternrecorder[9984]: C thread_unknown MythExternRecApp.cpp:81 (config) ini file missing [RECORDER]/command

So not specific about mythexternrecorder not being able to open the configuration file at all.

I really don't speak C++ very well at all, so it is not obvious to me where the configuration file is actually being opened/parsed. Still, I'll keep looking at it in an effort to propose a simple patch that sends a STATUS message to the backend if the configuration file doesn't exist.

Thanks, Jan

jpoet commented 1 year ago

I just pushed a fix of mythexternrecorder such that it will report to mythbackend when it cannot find the config file.

JanCeuleers commented 1 year ago

On 11/12/2022 23:07, John Poet wrote:

I just pushed a fix of mythexternrecorder such that it will report to mythbackend when it cannot find the config file.

— Reply to this email directly, view it on GitHub https://github.com/MythTV/mythtv/issues/670#issuecomment-1345670245, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC2CDDYDMYIZK5BWQZCD6STWMZGCXANCNFSM6AAAAAASMPEETE. You are receiving this because you authored the thread.Message ID: @.***>

Thank you very much