search

MythTV / mythtv

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

Playing a video, critical errors are logged every 5 seconds #483

Closed rcrdnalor closed 2 years ago

rcrdnalor commented 2 years ago

What steps will reproduce the bug?

When playing a video, every 5 seconds the critical error is logged: C ProgramInfo(): Failed to find recorded entry for 0.

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

Every time a playback of a video is requested

What is the expected behaviour?

Playback without critical errors

What do you see instead?

Playback of a video spams log file with critical errors

Additional information

I looks like 'ProgramInfo::ProgramInfo' https://github.com/MythTV/mythtv/blob/594a0170077123801d56bf735f6f6f3a4c990014/mythtv/libs/libmyth/programinfo.cpp#L235 is called without valid context/parameter.

If I change the timer value of 'kSaveLastPlayPosTimeout' in the file tv_play.h, then the frequency of the logged messages change according this timer value.

This is likely the culprit: https://github.com/MythTV/mythtv/blob/594a0170077123801d56bf735f6f6f3a4c990014/mythtv/libs/libmythtv/tv_play.cpp#L9965

Is the 'TV::HandleSaveLastPlayPosEvent()' logic supposed to work with video files as well?

kmdewaal commented 2 years ago

This bug is introduced in commit c6569589a82d341f902cb30ddb3866c4f68a2924 which is part of issue Recording watching progress on recordings list #331 from warpme and linuxdude42. After this commit the position is saved on exit and only after 30 seconds playing, and then the message comes: 2022-02-15 20:43:41.395595 C ProgramInfo(): Failed to find recorded entry for 0. Later versions give this every 30 seconds automatically and then later the interval is reduced to 5 seconds.

linuxdude42 commented 2 years ago

Yes, the last played position is saved for videos as well. I have tried several different videos and it is working for me here. I do not see any error messages being printed. I will keep investigating.

rcrdnalor commented 2 years ago

Is the 'TV::HandleSaveLastPlayPosEvent()' logic supposed to work with video files as well?

Just tested this on master, it works for video playback as well: the video is played from the saved position if selected again. BUT there are no means in MythTVs Video Library by now, to select either video playback from the beginning or playback from saved position. I propose to hide this feature by now, at least for the upcoming release v32 .

linuxdude42 commented 2 years ago

I was looking in the frontend log earlier. I do see the error in the backend log.

kmdewaal commented 2 years ago

Code around line 1528 in mainserver.cpp is clearly a bug, there should be a check on recordid > 0 before creating the ProgramInfo.

linuxdude42 commented 2 years ago

Agreed, but that bit of code hasn't changed since 2015. I'm trying to find where/why the frontend is sending a recording id of zero,

kmdewaal commented 2 years ago

My 2 cents.. I think that videos are not recordings and do not have a recordedid. Videos are just files in a folder. Looks like the question is why the frontend sends a MASTER_UPDATE_REC_INFO at all when playing a video. This can only be from ProgramInfoUpdater::insert who is called now also for videos instead of only for recordings.

gigem commented 2 years ago

Well, my 2 cents are that it should not matter whether a file is the result of a TV recording or ripping a DVD or whatever. All reasonably applicable, MythTV functionality should work on it. I realize that MythTV's evolutionary nature makes this difficult, even very difficult, in some cases. In hindsight, the database schema should have been done quite differently to facilitate this. However, until someone get motivated enough to totally revamp the schema and all the code which uses it, we're stuck with what we have. In the meantime, we should try to do our best with what we have.

David -- David Engel @.***

linuxdude42 commented 2 years ago

The problem is that the video playback code "fakes" a ProgramInfo data structure so that it can use the same videoplayback code in tv_play.cpp that the recordings playback code uses. What changed in https://github.com/MythTV/mythtv/commit/c6569589a82d341f902cb30ddb3866c4f68a2924 is that the last position save which used to only update the ProgramInfo structure on the frontend, now also sends a notification to the backend that a ProgramInfo has changed. That notification is what is triggering the warning message on the backend.

linuxdude42 commented 2 years ago

BUT there are no means in MythTVs Video Library by now, to select either video playback from the beginning or playback from saved position.

This has been fixed too.