Expired programs not moved to deleted recordings group

[Ian Dall <ian@…>]

I understand that deleted programs are always supposed to be moved to the "deleted programs" group. Commit f78f9992d754390fa42f109e5139b8eaf224d076 is titled "Change delete behaviour so that we always use the deleted recording group". However this is not what the code does in the following cases:

1. where the master backend is also the recording host (common);
2. where the recording is LiveTV;
3. where the recoding is less than 1MB.

Attached is a patch to fix case 1. The expirer argument of MainServer::DoHandleDeleteRecording?() is no longer used so it is deleted also.

Maybe the exceptions for cases 2 and 3 (above) should be removed as well.

[Ian Dall <ian@…>]

Patch to eliminate "expirer" argument from MainServer::DoHandleDeleteRecording?()

[Ian Dall <ian@…>]

I should add that this fix pretty much depends on the fix to ticket #10704. This ticket is about making sure recordings go to the "Delete" group. #10704 is about making sure programs in the "Deleted" group do actually get removed.

[paulh]

There is a patch so hopefully someone will look at this for 0.27

[stuartm]

I'm not sure that I agree with the assessment of this patch. It claims to fix deleted recordings not going to the deleted recording group "where the master backend is also the recording host".

I not sure that's actually broken, but the exact circumstances of the 'bug' aren't described. I don't have any problems with recordings where the backend is the recording host.

The proposed fix forces expiring recordings to go to the Deleted group instead which is not what it's described as doing. I don't see the advantage in that, since the recorder expires files to create space, moving them to the deleted group would only delay the inevitable for a moment before they would be force deleted to create the necessary room.

There may be an issue with 'max episode' deletions not going to the deleted group, but this patch wouldn't be the right way to fix that.

[stuartm]

See [50b61762] and #3788 which introduced the fix to prevent expiring recordings being added to the Deleted recgroup.

[sphery]

Ref 580116731b6d693a647d5b2c3863e77421a0b480 , which ensures recordings that are deleted due to a recording rule's Max Episodes setting are moved to the Deleted recording group rather than immediately removed from disk.

Should this ticket be closed?

[ian@…]

See comment 1. Ticket #10704 was about fixing a real functional problem of the disk filling up due to "deleted" recordings not really being deleted. This bug was apparently introduced by commit f78f9992d754390fa42f109e5139b8eaf224d076. I also noticed that commit didn't really implement what was apparently intended according to the commit message: "Change delete behaviour so that we always use the deleted recording group".

I assumed that there was some reason for always using the deleted recordings group. I don't have a use case where this matters (perhaps refer to the originator of f78f9992d754390fa42f109e5139b8eaf224d076) but in anycase it does seem wrong that recordings made on a remote backend would have different rules than recordings made on the master backend.

I don't understand (comment 4), "which is not what [the patch is] described as doing" - I though that was exactly what the patch was described as doing - but no matter.

In summary, this was just an attempt to make the code do what appeared to be the original intent. I may have misunderstood the original intent and I don't have a strong opinion on whether recordings should go to the Deleted Recordings group first or not.

[sphery]

The comment, "always use the deleted recording group," means when the user is deleting recordings. Since auto-expiring recordings are being expired to make space available for current recordings, and expiration results in removal of both the recording file and all its metadata (including recording group) and is not reliant on the Deleted recording group/delete queue, there doesn't seem to be a benefit to changing the recording group of a recording immediately before expiring it.