-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Another Matroska Attempt #1149
base: master
Are you sure you want to change the base?
Another Matroska Attempt #1149
Conversation
Thanks for your efforts in moving Matroska support forward. This would really be a useful addition to TagLib. I would be really interested in this feature and want to encourage you to go on. But I think that before merging the code, it should also support writing metadata. Reading and writing should work for the elements which are commonly used in Matroska/Webm files. The problem here is that I do not have any mka files, and the mkv files I have are mostly grabbed from DVD using Handbrake. So probably it is a bit hard for me to find out which elements are commonly used. Before looking at the code, I wanted to get an overview of what works. I created a tiny mkv-file for test purposes using
When I use
This is also what I see below
I can see this information using Now I tried to add some tags using VLC. Unfortunately, this does not seem to work anymore (see bugreport). VLC is using TagLib, maybe this bug will not be fixed until TagLib supports Matroska. Then I tried Handbrake by converting my tiny mkv file to another mkv file and setting the tags in the Handbrake UI.
Also
This already looks promising! What seems to be missing is the title, in this case "handbrake", which is shown by
This is exactly what is contained in the corresponding tag XML file.
|
Updates:
The current code always rewrites the file when the new tag is a different size from the old tag size (which is usually the case). This can be slow with large file sizes. I will eventually optimize the writing to intelligently use void elements as padding to avoid rewrites when possible, similar to what is done for other file types. For generating sample/test files, I don't recommend using ffmpeg or any program based on it (such as Handbrake). ffmpeg's Matroska metadata implementation is not even close to compliant with the specification. Based on my research, Foobar2000 seems to be the only program with a decent Matroska audio implementation. So I'm using that as the reference implementation where the official Matroska spec is ambiguous. I plan to work on attachments next, for cover art support. |
Great, thanks! Concerning rewriting the file when the tag changes: Most audio format implementations in TagLib avoid this by using some padding (such as void elements). But I personally think that it is important to be able to bring the file back into a deterministic state without padding when the tag is completely removed, this should be possible with all audio formats supported by TagLib. You could implement it like this:
I will have a look at the changes. |
I had a look at the current state of the Matroska code, and progress looks promissing. I only have a few remarks: It gives me a good feeling if I can edit the metadata of a file and am then able to revert the changes thereby bringing the file into the exact state it was before. So I used Regarding the state of the file after the tags have been removed: It was no longer the same, the tags are removed, but the previously existant tags have been changed (Language from "Not set" to "und"). I would prefer if tags which are not edited would be kept as they were, so that the file is brought back into its previous state. I also had a look at how tags written by other applications are recognized. This looked quite good, only the year was displayed as 0. This is because foobar2000 uses DATE_RECORDED/30, Mp3Tag DATE_RECORDED/50, but tagwriter uses DATE_RELEASED/50. Given that TDRC in ID3v2.4.0 is "Recording time", I would also take DATE_RECORDED. Tags from Handbrake were still not recognized, probably because all tags from Handbrake have a "Target Type Value: None". Maybe TagLib could be a bit more forgiving here. Concerning the code itself, it is too early to fix minor styling issues, but I have one remark. In matroskatag.h there are a few template functions in the public header file. Probably these "simple tag" template functions could be moved into matroskatag.cpp as members of TagPrivate, then they could directly access tags and would not need a But as I said, it looks promissing and I am looking forward to seeing further progress. |
Thanks for the feedback. I'm continuing to work on this locally. I have attachments working, but I have discovered some other issues:
If you run |
Conflicts |
@neheb I'm working on some changes locally to fix the issues I described in #1149 (comment). After I have that done, I will rebase before pushing again. |
@complexlogic I just want to ask if you are still working on this. If you have local changes, you could push them to the branch, so we could continue working on this feature. |
I have fixed the conflicts and pushed to a new branch https://github.com/ufleisch/taglib/tree/matroska. |
Since the other PR for Matroska (#967) hasn't had any progress in several years, I decided to give it a try myself. I think that the Matroska container format has a lot of potential for audio and would like to see it supported in TagLib.
Matroska is different from other metadata formats in that it uses nested/hierarchical elements to store data. I designed an API which internally parses the nested EBML structure and generates a flat list of tag objects to present to the user. This allows Matroska files to be parsed in a similar manner to the other formats in TagLib.
The user can add/remove tags to the list, and manipulate the tags' attributes (for example, the target level). When the file is written, the nested EBML structure will be re-rendered from the flat list of tags. None of the EBML-related related code is part of the public API.
I created the programs
examples/matroskareader.cpp
andexamples/matroskawriter.cpp
to demonstrate how the API works.Current Development Status