Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#7731 closed patch (fixed)

Delay file deletion in mythtranscode

Reported by: stichnot@… Owned by: cpinkham
Priority: minor Milestone: 0.24
Component: MythTV - Mythtranscode Version: Master Head
Severity: medium Keywords:
Cc: Ticket locked: no

Description

If you're watching a program that is being transcoded, the unlink of the original file at the end of the transcode causes the player to exit ungracefully, and you lose your position in the playback.

This simple patch delays the unlink, and other subsequent cleanup, until the recording appears to be otherwise not in use.

Note that this patch does not change the preexisting odd behavior of resetting the recording's bookmark and watched flags when a cutlist is present.

Attachments (10)

delay_unlink.patch (1.8 KB) - added by stichnot@… 10 years ago.
delay_unlink_fix_bookmark.patch (9.2 KB) - added by stichnot@… 10 years ago.
fix_bookmark.patch (5.2 KB) - added by stichnot@… 10 years ago.
delay_unlink_2.patch (6.3 KB) - added by stichnot@… 10 years ago.
delay_unlink_fix_bookmark_2.patch (9.8 KB) - added by stichnot@… 10 years ago.
delay_unlink_3.patch (5.6 KB) - added by Jim Stichnoth <stichnot@…> 10 years ago.
fix_bookmark_3.patch (4.4 KB) - added by Jim Stichnoth <stichnot@…> 10 years ago.
delay_unlink_fix_bookmark_3.patch (9.8 KB) - added by Jim Stichnoth <stichnot@…> 10 years ago.
delay_unlink_fix_bookmark_5.patch (5.8 KB) - added by Jim Stichnoth <stichnot@…> 10 years ago.
delay_unlink_fix_bookmark_7.patch (5.8 KB) - added by Jim Stichnoth <stichnot@…> 10 years ago.

Download all attachments as: .zip

Change History (23)

Changed 10 years ago by stichnot@…

Attachment: delay_unlink.patch added

comment:1 Changed 10 years ago by stichnot@…

At Mike Dean's challenge, I updated the patch to also do something sensible with an existing bookmark when a cutlist is present, rather than just clearing it. Computing the bookmark for the new file based on the old file's bookmark and cutlist is straightforward. Things are somewhat complicated when a bookmark is updated while mythtranscode is waiting to delete the original file, because it may be hard to know whether the bookmark was set with respect to the old or the new file.

Here is how it works at a high level:

  1. There was existing code that updates the recordedmarkup table to remove commflag and cutlist marks. This has been moved to a point before the original recording file is deleted, so that if a new playback instance is started on the new file while waiting to delete the old file, the new playback instance won't have the old cutlist. (An old playback instance will consequently have its cutlist removed, but that's preferable to having a bogus cutlist for a new playback instance.)
  1. Before waiting for old playback instances to exit, reload the bookmark (since it might have changed during the several minutes transcoding takes) and update with the translated value so that new playback instances have the right bookmark.
  1. Take an initial timestamp, then periodically (every 10 seconds in this code) check the inuseprograms table for any 'playback' records that were updated before the initial timestamp. If there are any, we need to keep waiting. When there are none, we can finally delete the old file.
  1. Reload the bookmark. If it has changed since step 2, translate it again and write it back. (Unfortunately, the new bookmark could be from a new playback instance in which case it shouldn't be translated, but there's not much we can do when both the old and the new files are in use.)

Changed 10 years ago by stichnot@…

Changed 10 years ago by stichnot@…

Attachment: fix_bookmark.patch added

Changed 10 years ago by stichnot@…

Attachment: delay_unlink_2.patch added

Changed 10 years ago by stichnot@…

comment:2 Changed 10 years ago by stichnot@…

Please ignore the first two patches in this ticket. The last three patches attempt to separate the two mostly-independent changes here: delaying the delete of the original file (delay_unlink_2.patch), and preserving the bookmark after transcoding with a cutlist (fix_bookmark.patch). Unfortunately, applying the two individual patches in either order causes conflicts with the second patch. This is because one patch moves a chunk of code several dozen lines away, and the other patch slightly modifies that chunk of code. For this reason, I also included a combined patch (delay_unlink_fix_bookmark_2.patch).

Changed 10 years ago by Jim Stichnoth <stichnot@…>

Attachment: delay_unlink_3.patch added

Changed 10 years ago by Jim Stichnoth <stichnot@…>

Attachment: fix_bookmark_3.patch added

Changed 10 years ago by Jim Stichnoth <stichnot@…>

comment:3 Changed 10 years ago by Jim Stichnoth <stichnot@…>

Updated with *_3.patch files. Disregard older files.

The original delay_unlink_fix_bookmark patch had a problem in which the file size could be updated in the database (due to truncation from a cutlist) while the original file was still being played. In this case, playback of the original file would exit prematurely when it reached the point corresponding to the new file length. Worse, it would not set a bookmark on exit because it thought it was at the end of the recording.

The new patch delays CompleteJob?() until *all* playback instances of the recording have exited. This simplifies things because we don't need to support simultaneous playback of the old and new files (though there is a very small window that could still allow simultaneous playback).

The delay_unlink_3.patch and fix_bookmark_3.patch files are now independent. I have also provided a combined delay_unlink_fix_bookmark_3.patch for convenience.

comment:4 Changed 10 years ago by ajlill@…

I (and some others) have noticed a problem where commercial flagging doesn't flag the last 10 or so minutes of a program. On my system transcoding and commercial flagging run in parallel, and transcoding is slightly faster than commercial flagging. If transcoding finishes before commercial flagging, will the commflagger see a premature end of file and stop? Can your patch be extended to wait for running commflaggers to finish as well?

comment:5 Changed 10 years ago by cpinkham

(In [23415]) Fix a regression in the JobQueue? from [21369]. During the QMap key conversion from chanid+starttime to jobID, the JobQueue? lost the ability to check for another job running for the same recording and multiple jobs could be running at the same time for a given recording. This patch fixes this regression so that multiple jobs for a given recording are run serially instead of in parallel.

Also add a locking in some places around reads of the runningJobs QMap. These haven't been there since the beginning of the JobQueue?, but we need to protect our reads to make sure we're not reading while the QMap is being updated in another thread.

References #7731 since the regression above was mentioned in this ticket.

Changed 10 years ago by Jim Stichnoth <stichnot@…>

comment:6 Changed 10 years ago by Jim Stichnoth <stichnot@…>

Updated with delay_unlink_fix_bookmark_5.patch. This is the combined patch, and I can separate it out into its two components (delay deletion of the original file, translate bookmark against the cutlist) if someone wants.

The patch has been simplified after [23886] moved file deletion into the backend. Since file deletion now appears almost instantaneous to mythtranscode, there is no longer any need to swap the order of file deletion and database update.

Changed 10 years ago by Jim Stichnoth <stichnot@…>

comment:7 Changed 10 years ago by Jim Stichnoth <stichnot@…>

Updated with delay_unlink_fix_bookmark_7.patch after the ProgramInfo? refactoring.

comment:8 Changed 9 years ago by robertm

Owner: changed from Isaac Richards to cpinkham
Status: newassigned

comment:9 Changed 9 years ago by cpinkham

Resolution: fixed
Status: assignedclosed

(In [26212]) When finished transcoding and the original file is being played back somewhere, wait for playback to complete before continuing with renaming or deleting the original out from under the player.

Also, preserve the bookmark when applying a cutlist by calculating the new bookmark position based on the portion(s) cut out.

Closes #7731 using patch by Jim Stichnoth.

comment:10 Changed 9 years ago by cpinkham

Milestone: unknown0.24
Resolution: fixed
Status: closednew
Version: unknownTrunk Head

Reopening this ticket for now as there has been a report that the bookmark position was not updated correctly by the new code.

comment:11 Changed 9 years ago by robertm

Status: newassigned

comment:12 Changed 9 years ago by cpinkham

Resolution: fixed
Status: assignedclosed

(In [26459]) Fix an issue with the bookmark being translated when a cutlist was applied during transcoding. The previous code worked fine when the cutlist was passed in on the command line, but not when the cutlist was retrieved from the DB. This allows the transcoder to pass the retrieved cutlist back to the caller so the caller can calculate the new bookmark.

Closes #7731 which I had reopened until I tracked this issue down.

comment:13 Changed 9 years ago by cpinkham

(In [26590]) Fix another buglet related to [26212]. If the first cut section is defined only by an end cut point and does not have a starting cut point set explicitly at frame 0, then the bookmark was not being adjusted correctly.

References #7731.

Note: See TracTickets for help on using tickets.