Fixing Track prop in matroska to work with other software

[TheMinefighter]

TRACK in Matroska was previously set incorrectly, making this property unreadable for VLC and others. This Fixes it by explicitly reading and writing PART_NUMBER on level 30 as described in the Matroska spec.
Before:

After:

(comparison based on unit test)

[Starwer]

I would hold back with this change.
I verified it again, the tag file generated currently with the TagLib test has a correct Matroska-compliant structure. Cf screenshot:

The file contains a Tag with an empty Targets to span to all file, as described in https://www.matroska.org/technical/elements.html
See also example in https://www.matroska.org/technical/tagging-video-example.html (last one: One file per episode).

Pity that VLC doesn't understand that. Now the proposed fix seems to work ok with VLC but not completely, as you may see that the total number of tracks is not displayed.
Moreover, you map the tracks in TagLib# always to 30 (TRACK/Chapter), and create a new Tag EBML to represent that, which doesn't necessarily represents the actual file. This is not so wise for a video. The Track property from TagLib would fit more to 50 (Episode). I don't know so many video-files only containing a chapter...
The idea of using an empty TargetType is that you don't have to assume what level the Track property shall represent. But for sure, this way, the Track property always spans the file, which is consistent with the rest of TagLib# (other formats).

So sorry, I'm not against pleasing VLC, but I'd like to be sure we do this following the Matroska and TagLib# philosophie.

[TheMinefighter]

@Starwer Thanks for your response, I have some thinks to clarify:

1. I didn't and didn't intend to fix the track count, that might be subject of another PR (after this has been merged), but i found the track number itself to be more essential.

2. The behavior implemented by me is compliant with the matroska specification. PART_NUMBER shouldn't be tagged globally but explicitly on TRACK level (for music when it's supposed to indicate a track number). This is made clear multiple times in the spec
   "PART_NUMBER - Number of the current part of the current level. (e.g. if TargetType is TRACK, the track number of an audio CD)"
   "So if it’s TRACK (30) then that means it contains 10 tracks"

3. Concerning video tagging you have a fair point, when creating this PR I didn't really think about video tags. Feel free to change the code from CHAPTER to EPISODE.

4. This is not only about VLC, but other software (Mp3Tag for example) too.

[Starwer]

I analyzed the problem further...
The problem is that VLC will show track number only on the Matroska TRACK level (30). This may sound like ok, and it is for musics. However, for videos, this is not so trivial. As already mentioned, a video file will most likely represent a MOVIE, or EPISODE (level 50), and not a track. As a result displaying a literal track-number make no sense. It would be more interesting that VLC show the EPISODE number instead, which it doesn't do.
Another thing that VLC does wrong: it shows the comment from the Tag representing the audio part of the video (A Tag with UID), whereas it shouldn't. But ok, it might sound silly to just tag the audio part, although this is perfectly Matroska-Valid.

Another thing: the way you "fixed" the issue may force to create a new Tag to contain TRACK, whereas the current Tag (representing the file as a whole) should be used to store/retrieve track-number.

My conclusion:

• The way it is currently implemented is valid for videos;
• maybe a sample video with a Tag on audio track is too tricky for VLC or other players to handle (although it is valid);
• The proposed fix is not ok for video
• we should try on an audio file whether current implementation requires a fix or not

[TheMinefighter]

@Starwer would you be okay if I change the behavior just for audio in a new PR an dclose this?

Another thing: the way you "fixed" the issue may force to create a new Tag to contain TRACK, whereas the current Tag (representing the file as a whole) should be used to store/retrieve track-number.

I don't really understand what you are trying say here; when there is no tag on level 30, of course I have to create one to store the tracknr. The same usage of TagsGet exists often throughout the file. What's wrong with that?

[decriptor]

@Starwer Could you follow up on this?

[Starwer]

Sorry, I'm still unsure about the proposed fix for Mka (Matroska audio) files here. It would greatly help if an unitary test was added to specifically check the support of Mka file, so that I could verify the created tags are ok.
If @TheMinefighter could add a test to this change, it would speed up the process. Otherwise I still need a couple of weeks before I could spend time on that.

[TheMinefighter]

@Starwer Just wanted to confirm, you mean that I add a unit test, which checks whether a Track Number (for Music) is correctly set in a Matroska file?
If so, yes I could do that.

[HomerJau]

I started a discussion thread on Matroska song tagging. Please take a look.