Bugfixes to mythburn.py

[t.brackertz@…]

A lot of fixes to mythburn.py No other files affected.

The path-file applies to the recent (2011-10-01) git-clone but changes can easily be backported to 0.25 branch.

See below for a detailed description of the issues and the fixes:

---

---

typos

---

some irrelevant ones

Medium handling:

---

former issues:

• no possibility to cancel process
• one has to be fast enough feeding the device, otherwise the process cancels
• some devices need more time to close the tray. They get hard-resetted all the time. No chance to burn a disk, if it is not put in before the script is started
• if you use a disk of wrong type you don't get a chance to put in a correct one. The whole process cancels

changes in BurnDVDISO()

Integration of project-x

---

until now: After the run of project-x its logfile is parsed (in renameProjectXFiles()) to find out the filenames of the files produced by project-x and to associate them to the streams

issues:

• not very reliable: in many constellations (e.g. recoded .flv-videos) the logfile doesn't fit the expectations. Although there is a fallback mechanism in many cases the script crashes while parsing before reaching the fallback
• the format of the logfile seems to change often if there is a new version of project-x. The parser has to be renewd all the time. (the recent one works for project-x 0.91 but not for older ones)

solution:

• An 0.91 project-x learned a new command line option (-set ExternPanel?.appendPidToFileName=1) which causes project-x to use filenames for the output containing the PIDs and SubPIDs in a predictable manner. This should work with upcoming versions, too.
• Therefore no more parsing is necessary.
• Nevertheless there is still a fallback mechanism as in some cases (e.g. recoded mggs) mytharchivehelper gives strange IDs which makes it impossible to find the wanted files by ID
• As the current parser relies on project-x 0.91 as well there is no regression by the fact that at least version 0.91 is necessary now. (The version dependency at the beginning of the sript claiming for project-x 0.90.4 was outdated anyway.)

changes in runProjectX(), renameProjectXFiles() is obsolete now

Divide-by-zero-error if requantising

---

see: ​http://code.mythtv.org/trac/ticket/8598#comment:2

issue: If M2VRequantiser fails for some reason (probably it is not installed) the script crashes with a missleading error message.

reason: The exit-code of M2VRequantiser is evaluated too late: After then run the size of the produced file is evaluated which gives a divide-by-zero-error. The exit-code of M2VRequantiser is evaluated thereafter, which means never because the script crashes before.

solution: change the order

changes in runM2VRequantiser

Misleading log-entry

---

issue: The log-entry produced by getStreamInformation() sometimes mentions the wrong file as the filename is hard-coded

solution: use the appropriate variable

changes in getStreamInformation()

Inconsistent string-handling in the whole script

---

issue: many errors and crashs when sript is used with recordings having special characters in their title or videos with special chars in the filename.

reason: One part of the strings is unicode (mainly coming from parsing the xml-files), another part is utf-8-encoded (mainly coming from database). They get mixed up and / or get printed to logfile or console unencoded or double-encoded. This results in many errors.

solution: Consequently use unicode-strings:

• make the database-object use unicode-strings in getDatabaseConnection()
• encode strings to utf-8 before printing to logfile or console in write() and runCommand()
• remove all encode- and decode-statements in the rest of the script
• quote all commandline arguments properly

Endless growing work-directory

---

issue: In some rare cases in which many DVDs with many small and one big title are created the work-directory theoreticallly grows endless

reason: Before a new file is written to disk the old file with the same name is deleted but the rest is not. Imagine the following situation: Create a DL-disc with 20 titles, all very small, only the last one having 8GB. You get 20 subdirs in the work-directory of which the "20"-directory is very big. All together they have 9GB. Next step: The same as before but with only 19 titles. Result: Only the first 19 dirs of the last run get deleted, the big "20"-dir stays. But there is a new big "19"-dir. All together they have ~17GB An so on. You end up with a work directory containing at least 160GB

solution: delete everything in the "work"-dir at the beginning instead of incrementally deletion. Also shortens the code.

new deleteEverythingInFolder()-function and changes at some other places

[t.brackertz@…]

Path file to apply the pathes to the recent git-version of the 0.25 branch

[t.brackertz@…]

backported fixes for 0.24-fixes-branch

[J.Pilk@…]

Unified diff from 0.24-fixes mythburn.py combining this with other patches

[J.Pilk@…]

I'm attaching a unified diff, from the 0.24-fixes version of mythburn.py, that combines the patch submitted above with modifications that I have been using for some time.

My changes are principally those from Tickets #8376 and #9389 that have not already been committed; mostly the remaining earlier ones are intended to allow processing to continue when a quite rare error has been signalled by mplex but experience has shown that no bad effects are likely to be noticeable, and the later one applies cutlists in the correct sense after the changes made between the 0.23 and 0.24 releases. Others are intended to make the log files more informative and to reduce the effect of often intensive disc activity on other processes.

I have not conducted an exploration of the effect of this combined patch on all possible pathways through the script, but with it my system now appears to behave during my normal mode of operation just as it did before the patches were merged. I have however used it to make, from UK dvb-t recordings, iso images with subtitles and with more than one audio stream, using an active cutlist; these are not things that I had recently tried to do. Since I work with UK-targeted material I have not had to contend with 'exotic' character sets and I have done no tests on this aspect of the new patch.

A few specific points should be made:

Most disc-intensive operations are done with the 'ionice -c3' prefix to reduce system impact. This may not be available for all OSes; I couldn't find it for CentOS5

The location of the output iso file (isopath) has to be specified in the script. Creation of the file is essentially a multi-GB file copy operation. Its impact should be less if the mytharchive temp directory and isopath are on different spindles.

The mytharchive temp directory will change in size by several GB during operations. This could affect auto-expire calculations if it shares a recordings partition.

I use mp2 sound, but mytharchive default is ac3. The patched script will look for menumusic.mp2, which AFAIK is no longer distributed. I converted it from menumusic.ac3 and it is selected by a hack that is not theme-aware. Disable by omitting chunks of the patch that mention menumusic.mp2

The patch was created by diff -Nau mythburn_dist.py mythburn.py > ~/mythburn20111006.patch

[J.Pilk@…]

WARNING: I have been using the combined patch, mythburn20111006.patch, since I submitted it, but have only just realised that the animated snapshots in the menus created with the MythCenter?-Animated theme that I use are not playing with any of the players that I have tried. Only the empty frames are displayed. DVDs created earlier do not have this problem. It's probably related to the double quote marks generated by the first patch above, but more investigation is needed.

[J.Pilk@…]

Fixed: the change required is to remove the single quotes around the third %s below at around line 3533

command = "mytharchivehelper -t %s '%s' '%s' %d" % (quoteCmdArg(inputfile), starttime, quoteCmdArg(outputfile), frames)

[sandin@…]

Hi, I'd like to see this in upstream because mytharchive really cannot handle recordings with non-ascii characters in title/desc (it worked fine in 0.24 and earlier).

[t.brackertz@…]

That's great.

But wait a minute because during the work on the patch I did a few more changes that

• tidy up the code
• allow better scaling on multiple CPUs
• prevent mythburn from eating up all the available disk-IO

But I haven't finished testing yet. I hope I will have enough time on the weekend. Maybe you can answer my following questions till then:

• How should we handle sound format:

Until 0.24 the default behaviour was to leave the format as it is if it's mp2 or ac3 or to reencode everything to ac3 if explcitely chosen by user. There are two competing patches now: One by J.Pilk here in the ticket which allows to chose to encode everything to mp2 and one in commit c5aca5cf4f3690c95a7d4a4a64c7ec0f6ec56dce which enforces encoding to ac3 all the time. As the comment to the commit suggests the problem are DVDs with mixed sound formats. So my proposition would be to let the user chose the sound format but take care that there aren't different formats on the same DVD. Do you agree? I would integrate this stuff on weekend if you do.

• How should I supply the path:

Because of commit c5aca5cf4f3690c95a7d4a4a64c7ec0f6ec56dce all the diffs above are outdated. I plan to provide one big diff including all the changes to the current git. Is this appropiate or should the different changes get split? Maybe I can also add a backport.

[J.Pilk@…]

I see no good reason to use ac3 when all my recordings have mp2, usually at a higher bitrate, but the menumusic in the distro now is only ac3. It could of course be converted as needed - a few seconds instead of hours - or distributed again.

I'm still on 0.24-1 and have not been looking forward to rethinking this patch. It still works reliably and often for me - although I think I may have imported a few 'ionice -c3' instances from my original setup screens - using UK material.

One other point: the calculations for requantising don't allow for space required for subtitles, which I have never used; I don't know if the source file size is a good measure of the extra space needed on the disc.

[sandin@…]

I think that breaking the patch into small separate patches according to individual fixes/features would be better (shorter review time). On top of that obvious fixes may land in repository rather quickly and would not be blocked by (relevant) discussions about other features. But that means more work if you want to maintain several patches (ideally in form of separate tickets).

Concerning audio format it would be great if user should choose whether to keep original tracks or encode everything into mp2 or ac3 (to get everyone satisfied). Anyway someone from core developers could comment on this since I'm just thinking loud here :).

[J.Pilk@…]

Breaking up a big patch into smaller ones might make review easier but doesn't address the basic problem here: the lack of people able and willing to actually do the review of patches for MythArchive? and perhaps drive it forward. The current 'owner' doesn't use it now and has other project(s) that I'm sure he finds more rewarding. I believe other devs feel the same. I prefer to use the 'fixes' branch and don't have much interest in testing paths through the code that I have decided not to use. If you have patches that you believe will be useful to others, please make them available as best you can and maybe they can get committed eventually. I would like to see them soon! I think that's called my 2c's worth; it's probably also a recipe for chaos if taken too far.

[paulh]

Thanks to everyone who is working on this.

I am the only dev who maintains MythArchive? all or most of the others want it removed :( which would be ashame but since I rarely use it myself I can't justify spending long periods of time on it. At this stage I'm not bothered whether it's a single patch or several so long as I don't have to spend a lot of time fixing things or working out why something was done the way it was.

I switched to always using ac3 audio because when I tested it around Christmas the internal Myth player couldn't handle the switching from one format to the other properly. Actually to be compliant with the ntsc dvd spec all the audio should be ac3 anyway. Changing the behavior should be as simple as changing encodetoac3 = True to False at the top of the script.

Just looking at the last patch added remind me of why it has sat here rotting. When I see stuff like

isopath="/mnt/f10store/myth/arctmp/isos/mythburn.iso" # is not stored in the db

something that clearly doesn't belong in the patch. It just adds to the time required to review it because it means having to spend time figuring out why was it added and and can I just remove it etc.

Something else that worried me when I first looked at the patch was the switch to the latest version of ProjectX. Generally if all the major distributions are shipping with that version then that is fine but if not then that can cause problems. If possible we like to wait at least 6 months after all the major distros start shipping new versions to give time for people to upgrade. It needs someone to do a little research to find out what version most distros are shipping with these days.

Other things to watch out for is fixing things so it works well for one locale or set of uses but breaks things for others. One common one is forgetting that not everyones recordings are in a TS container some are in PES. ProjectX for example behaves differently depending on the format you pass it.

[J.Pilk@…]

A quick response here: I added the isopath definition because I wanted to shift the destination and couldn't see a better way of doing it without the need to recompile something else or hijack an existing variable. I think the idea is valid; the implementation is a hack.

[t.brackertz@…]

As mentioned above I'm going to provide a clean path the next days without any user specific hacks (and a few additional bug fixes).

Audio:

I'm going to implement encodetomp2, too. (Hopefully this will get implemented in the UI, too.) In each case (encodetoac3, encodetomp2, none of both) the audio format will be the same for the hole disc which means that the menumusic is also reencoded if needed. As NTSC players don't need to accept mp2 encodetoac3 will be enforced if the disc is NTSC.

ProjectX:

In general I agree to your argument, paulh, but:

• The old mechanism doesn't work in many cases as the output-filename chosen by projectX varies a lot not only depending on the input but also on the specific version. The old fallback-mechanism doesn't work correctly and additionally there is a bug terminating the hole script even before the fallback starts. Unfortunately this happens very often.
• The new version of projectX allows to exactly specify the desired output-filename-sheme by a new command-line-option. Therefore we get rid of all the trouble.
• If this doesn't work there is a robust fallback-mechanism which guesses a working result in each case. The worst thing which can happen is that different audio-languages get exchanged. This unfortunately happens sometimes because in some cases mytharchivehelper gives wrong IDs. (I haven't investigated the reason yet.) The old mechanism gets confused in this cases, too. The new fallback also works for old versions of projectX which can't handle the new command-line-argument.
• The new projectX is available for many distros at least by third-party-repositories and can therefore easily be obtained.

@paulh: I (and maybe not only I) think it would be a pitty if mytharchive would get removed. Maybe I can support you in maintaining it. What do I have to do to get a (restricted) git-account? How do I need to show that I'm reliable and competent enough? (Maybe I could not make things much worse if you would completely remove mytharchive otherwise.)

@J.Pilk: I'm going to introduce an additional variable for the destination of the iso-file. You can set this in the beginning of the script until it is implemented in the UI. But I agree to paulh: We shouldn't have specific paths and stuff like this in the git and the patches here. Hint: I have a MBE/FE-machine with 4 harddisks for storing the records. I have split off a few GB of each harddisk and bulit a raid0 on that. Using this as working directory of mytharchive speeds up the process *a lot*.

[J.Pilk@…]

This all sounds good, and maybe I should investigate raid. I introduced 'isopath' and similar ways of controlling disc use in eg mythcutprojectx because atop showed the discs working near their max rate while the cores had spare capacity.

I hadn't looked recently for binaries of Project-X or M2VRequantiser; for me, both have built easily from source as linked in the existing patch.

As hinted in Comment 6 above, I'm not now using the patch exactly as it appears here, but most of the differences don't affect it in use; just a few extra print commands to help the debugging that led to the additional and necessary patch of Comment 3.

[paulh]

Replying to t.brackertz@…:

@paulh: I (and maybe not only I) think it would be a pitty if mytharchive would get removed. Maybe I can support you in maintaining it. What do I have to do to get a (restricted) git-account? How do I need to show that I'm reliable and competent enough? (Maybe I could not make things much worse if you would completely remove mytharchive otherwise.)

I'd be happy for you to help maintain it. You would have to prove yourself by providing a few patches to show what you can do before you would be considered for commit access. Then it has to go to a vote from all the other devs.

[t.brackertz@…]

Here is a new path containing a few more fixes regarding non-ascii-filenames and drive-handling. It applies to the recent git (1.3.2012) and doesn't contain any user specific hacks.

It does not contain the audio-changes I have anounced before. Like the version in git it always esforces ac3.

This is because during the changes I encountered some problems (like nuv-files getting cut twice and so on). In addition the code is in parts a bit circumstancial and redundant and tries to use (myth)ffmpeg-options which don't any more. Additionally the process could be speed up if different files get processed parallel as some steps do load cpu, others harddisk. Taking all this into account I started a bit comprehensive refactoring which I haven't finished, yet.

Though I'm willing to continue the next days I think the chances to get the result into 0.25 are low - especially as we have feature-freeze now. Therefore I provide this path first hopeing that at least the most important fixes get into 0.25.

[t.brackertz@…]

All fixes mentioned before except the ones concerning audio-formats

[t.brackertz@…]

All fixes mentioned before except the ones concerning audio-formats - error in last file, disregard it

[t.brackertz@…]

again the wrong file - sorry

[paulh]

Thanks for the patch, sorry to be a pain but could you add the patch as a unified diff they are easier to read and see what you changed. IIRC its 'diff -u' or I 'git diff' will by default produce a unified diff.

[t.brackertz@…]

unified version

[Github]

MythArchive?: various minor mythburn.py script improvements

• quote all filenames passed to external commands.

• use the feature available in later version of ProjectX to add the PID of the stream to the created files to help in choosing which files to use.

NOTE this requires ProjectX >= 0.91 to work properly it will fall back to guessing which files to use so should still work with older versions.

• rework DVD burning to keep trying to successfully burn a DVD prompting the user to insert a blank DVD until success or the user cancels the script.

• plus a few other minor fixes.

Closes #10071.

Signed-off-by: Paul Harrison <pharrison@…>

Branch: master Changeset: b85f5bba7294faff8cacad4eb59caa72c49be801

[roben <rolf@…>]

There is a bug in the patch. Please see mythburn.py_fix_for_git_3.12.12.patch.

[paulh <mythtv@…>]

This fixed in my fork thanks

​https://github.com/paul-h/mythtv/commit/1536ff9606

[Paul Harrison <mythtv@…>]

In 146b49029d82e6ad92ac36ef66fe14d5c4c998e0/mythtv:

MythArchive?: fix a filename quoted twice bug introduced in b85f5bba72

Fixes #10071.

[Paul Harrison <mythtv@…>]

In d6de041b35358184841200da19ab160e15a287e1/mythtv:

MythArchive?: fix a filename quoted twice bug introduced in b85f5bba72

Fixes #10071.
(cherry picked from commit 146b49029d82e6ad92ac36ef66fe14d5c4c998e0)