getTagger holds file open after use

[mayfields@…]

When performing a music scan, last file scanned is kept open even after applying the various fixes from 10766.

When the portion of AddFileToDB which deals with embedded images is commented out, the file is no longer held open. The specific offending call is

MetaIO *tagger = data->getTagger();

If the lines after this are commented out, the issue still exists. If this one additional line is commented out, the file is no longer held open after the scan.

The tagger as defined within getTagger is a static MetaIO type eg

static MetaIOID3 metaIOID3;

I have only seen this particular behaviour with the ID3 tagger which holds the current file open until the next is read. I am unable to determine a mechanism to force the file closed. File is only closed upon mythfrontend exit.

I have not confirmed whether this issue exists where other calls to getTagger are made.

[mayfields@…]

The tagger needs to have the file open. However, there are a couple of issues with this.

The first is that when a flacvorbis tagger is returned, the ID3 tagger has the file open as the ID3 tag is checked first. This should be closed prior to returning the flacvorbis tagger.

The second is that, once used for whatever purpose, the tagger is not closing the file. There needs to be a mechanism for the tagger to close the file.

I will put a patch together that addresses both of these and attach, but it may not be the ideal solution as it will simply attempt to read metadata from a non-existent file by passing NULL as the filename which will fail in all cases but force the MetaIOID3 tagger to close the current file. This will likely lead to spurious errors about files not being able to be read, even though it is the intended behaviour.

If there is a way to determine which class the tagger is then the preferred option would be to determine the class and only call tagger->read(NULL) if the class is MetaIOID3. This will fail without error as it checks the file extension.

[mayfields@…]

Close file after using getTagger()

[mayfields@…]

Partial re-write of MetaIOID3 to close file after use

[mayfields@…]

2 patches attached. Each addresses the issue in a different way.

patch.lastopenfile.1 calls MetaIOID3::read() with a NULL argument after each use of the tagger, once the tagger is no longer required. This forces the file closed.

patch.lastopenfile.2 is a partial re-write of MetaIOID3 to close file immediately after use each time. This more closely matches the other metaIO classes which all close the file each time.

Each patch has been tested with file import using FLAC and mp3 files in various scenarios. Other aspects have not been tested. Beyond that case no further testing conducted. Written and compiled against 0.25-fixes but should also compile against mythtv-master.

I do not know whether it is desirable to fix this behaviour or whether it is intended. There may be a reason I am not aware of to keep the file open, beyond the elegance of preventing the opening and closing of a file multiple times to read or write metadata details.

Use cases where this may be a problem is with users who may plug in a portable music player or other external storage device or network storage containing music and scan this in order to play music, intending to unmount it after use. Keeping the file open may either prevent unmounting or lead to filesystem errors due to unmounting (or simply unplugging) with files in use. There may be a reasonable assumption that no files are in use after exiting the music screens. However, as the file is only finally closed at mythfrontend exit this assumption would be incorrect.

[paulh <mythtv@…>]

See ​https://github.com/paul-h/mythtv/commit/93def80c52 for a fix.

It just creates the taggers as required, there is very little benefit from reusing the same tagger each time since most of the time the file has to be closed and another one opened anyway.

[Paul Harrison <mythtv@…>]

In 212b53ed28d25414ae6a77bdbbc205aa10362500/mythtv:

MythMusic: Move all the tag reading/writing into the MetaIO classes

This removes the tag stuff from the decoders and moves it into MetaIO where it
belongs.

Fix a bug if a file had an upper case file extension. Fixes #11177

Fix a problem when reading id3 tags with files being left open. Fixes #10778