Commercial Flagging isn't being queued.

[Bill Meek <keemllib@…>]

In: TVRecEvent programinfo.cpp (GetFilesize) the existing returned filesize is sometimes 0 and TVRec::FinishedRecording won't allow the job to be queued if the file size is less than 1000 bytes.

This has deactivated all commercial flagging. A workaround is attached (not a fix.)

[Bill Meek <keemllib@…>]

Workaround - tested on: v0.28-pre-2261-g53254f3

[km@…]

Thanks to Bill Meek for the workaround. I've had to use it to patch programinfo.cpp every time that file gets updated on the trunk for the last 5 months. Isn't it about time to integrate it?

[stuartm]

I believe this was fixed months ago.

[stuartm]

If this continues to be a problem for anyone then we need more information. The file size is not even stored in ProgramInfo any more, they are saved to and read from RecordingFile and the corresponding RecordedFile table.

[Stuart Morgan <smorgan@…>]

In 767370fe7ea41f78ae99b7cd1ef4ddcdf5d8d016/mythtv:

Error: Processor CommitTicketReference failed

GIT backend not available

[Bill Meek <keemllib@…>]

Sorry, that doesn't solve the issue.

I added some LOG()s to demonstrate.

tv_rec.cpp:936 (FinishedRecording) - fsize>=1000 allows previewgen. fsize=0.
...
tv_rec.cpp:989 (FinishedRecording) - fsize<1000 disables commflag/xcode. fsize=0.

Full log and LOG() patch attached.

[Bill Meek <keemllib@…>]

1st try blocked

[Bill Meek <keemllib@…>]

To answer the questions on the mythtv channel, it's not the 0 byte file case. The recording plays back OK, manual commercial flagging works OK

mysql> SELECT recordedid FROM recorded WHERE title='Love It or List It';
| recordedid |
|       2916 |

mysql> SELECT r.filesize,rf.filesize FROM recorded AS r,
    recordedfile AS rf WHERE r.recordedid=2916 and rf.recordedid=2916;
| filesize  | filesize  |
| 883567100 | 883567100 |

[stuartm]

Thanks for the follow up Bill. So it seems the cached RecordingInfo used in TVRec isn't being updated ...

[Stuart Morgan <smorgan@…>]

In cf4da125a78bde32fb309fbddfc557993d4045fe/mythtv:

Error: Processor CommitTicketReference failed

GIT backend not available

[Stuart Morgan <smorgan@…>]

In 31da15ad7b55cd203f0f9d86b68de4aae089b317/mythtv:

Error: Processor CommitTicketReference failed

GIT backend not available

[Bill Meek <keemllib@…>]

I added 2 more LOG()s and they return:

TVRecEvent recordinginfo.cpp:1628 (GetFilesize) - #12290 GetRecordingFile()->m_fileSize = 0
TVRecEvent recordinginfo.cpp:1637 (GetFilesize) - #12290 ProgramInfo::GetFilesize() = 0

And more LOG()s in RecordingFile::RecordingFile and RecordingFile::Save:

This keeps incrementing during the recording:

recordingfile.cpp RecordingFile::Save() #12290, m_fileSize=405709264

Which then starts returning 0 (1268 times) after the recording finishes

Full log attached.

[Bill Meek <keemllib@…>]

Additional LOG()s added.

[drew@…]

Can anything be done to further this along? This bug is quite annoying for those of us that don't build from source to manually patch.

If someone can point me where to start, I don't mind attempting a patch myself.

[Bill Meek]

POC patch, works fine, I just don't know how to treat the other 6 calls to FinishedRecording().

[JYA]

Bill, anything more required ? that last patch seems good enough to me.

[Bill Meek <keemllib@…>]

Unfortunately, it isn't enough. When RingBufferChanged() (​https://code.mythtv.org/cgit/mythtv/tree/mythtv/libs/libmythtv/tv_rec.cpp#n3358) is used, I've not been able to recover a size. I haven't seen the other 4 calls to FinishedRecording() used. Had to go back to the workaround in comment 1.

[km@…]

I can also report that the new patch is not a complete solution. Some recordings get flagged others don't. With the original patch all recordings that are set for auto flagging get flagged.

With neither patch, nothing gets auto flagged.

[Bill Meek <bmeek@…>]

In 2e98fb583276c3966fed21fab354b11c7e5d591c/mythtv:

Error: Processor CommitTicketReference failed

GIT backend not available

[Bill Meek <bmeek@…>]

In 4ffac871792a83aa354230a0655651c934c2bf40/mythtv:

Error: Processor CommitTicketReference failed

GIT backend not available

[Stuart Auchterlonie]

Moving to 0.29

[Stuart Auchterlonie]

Milestone renamed

[Roger Siddons]

The "Hack fix" (31da15ad7) wasn't implemented correctly.

There are 2 instances of m_recording_file (in 2 separate RecInfos): the TVRec one is created at start of the recording and never updated, the Recorder one contains the updated filesize (and other data).

That patch intends to reload the stale instance but it is ineffective because the TVRec instance already exists. It needs to be forcibly reloaded.

I reverted the 4 patches on this ticket (2e98fb5832, 31da15ad7, cf4da125a78, 767370fe7e) and applied Bill's instrumentation to reproduce the before.log. Then applied the new patch to produce the after.log.

[Roger Siddons]

Zero filesize

[Roger Siddons]

Correct filesize

[Roger Siddons]

Logging

[Roger Siddons]

Fix

[Stuart Auchterlonie]

Moving remaining open 0.28.1 tickets to 0.28.2

[Roger Siddons]

Can anyone confirm whether the solution in comment 21 fixes the issue ?

[Bill Meek]

Replying to rsiddons:

Can anyone confirm whether the solution in comment 21 fixes the issue ?

Roger,

Only two tests, and flagging is queued (and works.) That's with the four reverts plus your fix. I should track it for a few days as others have reported inconsistent results with previous patches.

Looks like an edge case when the backend is restarted mid recording. The 2nd recording is flagged, the 1st isn't. I don't recall ever testing this originally and haven't tried with the old solution yet. Likely a new issue.

[Roger Siddons]

Thanks Bill,

Soak-test by all means but it would be nice if we could tidy this up for 28.2

The "old solution" was a work-around to the fact that a new implementation wasn't working properly.

I don't actually use the commflagger. I landed here because I had an issue with 0 filesizes elsewhere. Turns out it was unrelated...

However, isn't the job created by an end-of-recording event ? Should the backend check for interrupted recordings due for comm-flagging on startup ? User should start it manually IMO. And outside the scope of this ticket :)

[Bill Meek]

Seems to be working great. Tested on v29-pre. Do you want to push the four reverts and fix or would you like me to?

[Roger Siddons]

On review, it appears to no longer be an issue on 28/fixes: the work-around seems to be effective.

Whilst the fixes in comment:21 fix the specific issue on this ticket, the underlying changes that provoked it remain incomplete so it's not robust enough to be back-ported.

Targeting it for 29/master, once I've assessed what is still broken & the way forward.

Leaving this open to track it. Please report any observations of wrong filesizes.

[Mac Michaels]

Is 3 years long enough to track this issue?

[Stuart Auchterlonie]

Bill, is this still an issue, or can this be closed?

[Bill Meek]

It's working. But I just undid a single conversion from Stuart Morgan's ProgramInfo -> RecordedInfo work. I see Roger kept it open to track the root cause.

Closing. A new ticket can be used to track the underlying issue if someone wants to go after it.