search

mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
27.93k stars 2.87k forks source link

Matroska playback #11780

Closed hubblec4 closed 9 months ago

hubblec4 commented 1 year ago

Hi all

I'm a EBML/Matroska developer helper and the author of chapterEditor.

I've been working with Matroska files since 2008 and since 2015 I've started to actively help in the development of Matroska, as well as in the development of MKVToolNix.

I'm a specialist in the Matroska Chapters/Tags as well as anything related to Matroska-Linking.

That's why I started programming my own chapter editor in 2013, because many (the most important and best) Matroska features are included in the chapters.

My goal is still to realize the Matroska Menu. A lot of time has passed and it is still not foreseeable when the Matroska Specs will be expanded accordingly.

But the biggest problem remains in the end, that a player supports all of that.

A while ago I set up a git repo that deals with playing Matroska files and the players. I also tested mpv and then didn't bother with mpv because too few features were supported. But I think mpv has the best chance of becoming a really good Matroska player. Especially because it looks very bad with Linux with good Matroska support.

But mpv has a wonderful thing that I have never seen with any other player. I mean scripting. I've started looking into it and I see all the possibilities. If I thought it through correctly, I could program a Matroska menu as a script completely independently.

However, mpv still has a few bugs and missing features and I really hope that I can find a team here that is willing to bring Matroska forward. I will be there with all my experience and help where I can.

I'm creating a checklist here for a better overview of everything I've already found in the Issues and PR's.

Hidden and Enabled Flag

The Enabled flag is indeed easy to handle. Such a chapter is fully ignored for non-ordered and ordered chapters. The Hidden flag is an important flag because it is used for some ordered chapters structures where you add content to the virtual timeline silently.

To create a multi-Edition mkv of a Blu-ray with multiple editions/movie cuts, it is necessary to use hidden ordered chapters. My chapterEditor is currently the only tool which can create such mkv's.

Nested chapters

This kind of chapter is indeed not widely used. Maybe because only few players support this. For all my Mainmenu.mkv's I use a Nested chapter structure, because it looks like a simple menu in the player(MPC-HC). All nested chapter names get a "+" in front for the first nested level and a "++" for the second level and so on. In a chapter menu (mostly a popup menu) it is now easy to find the correct chapter.

Nested ordered chapters

This topic is not really well defined in the specs. My suggestions how to handle this kind of chapters was rejected. At the moment my chapterEditor uses a workaround for that to simulate Nested ordered chapters. Unfortunately mpv don't handle this chapter structure correctly.

Hard-Linking

This is a popular kind of linking mkv's for users which don't want to use ordered chapters. My chapterEditor has an easy to use editor for this kind of linking. Only 2 Matroska elements are used for this feature and it is the weakest linking kind. Soft- and Medium-Linking "overrides" the Hard-Linking. But Hard-Linking has many "traps" and restrictions.

Only MPC-HC handles Hard-Linking correctly. VLC and other players have issues on the transitions and the video hangs sometimes a bit.

Native Matroska menu

This feature sounds worse than it actually is. A demuxer doesn't have to worry about the content of the data at all. Only the ChapProcess element has to be triggered, the rest then has to be managed by a Chapter-CODEC. Only VLC supports this and it works and is a nice feature. My chapterEditor also supports this.

Content grouping

This is maybe the best feature ever. With Haali's TRACKSETEX you can switch multiple things in one go. On Matroska is a discussion for implementing this feature.(but in a wrong direction). I'm sure this could be coded via script. Maybe I will try this as first, to learn more about scripting for mpv.

Can the mpv team imagine working on these Matroska features?

CogentRedTester commented 1 year ago

I can't help with implementing any of these features natively, but I could give you some tips and resources for implementing these features as scripts. You likely already know this, but I already have a script that implements hard-linking using mkvinfo.

General resources:

It should theoretically be possible to read and understand the mkv file directly in Lua without mkvinfo if you understand the encoding format. I believe mpv may have a python file which does this. The biggest limitation is if you want to read the metadata of a network file. Lua does not have a native API for downloading network files, so you will need to use external tools of some sort. There are probably default ones pre-installed on each platform that you can call through the command line.

If you do develop a way to read mkv header information from inside Lua please let me know.

Now for the specific features you've suggested:

Bonus:

If you make any attempts at implementing these features as one or multiple scripts feel free to ping me if you want any advice. I make no promises on the timeliness of my replies however.

hubblec4 commented 1 year ago

First many thanks for your reply.

I have studied your code for Hard-Linking and reading a lot of the mpv manual I understand a bit how mpv works and what can I do. But I'm sure there are many many more to learn.

My first idea was to modify your script to parse a Matroska file by my self which eliminates the mkvinfo depence, because I fully understand the Matroska semantic and structure. I know it is not so easy to write a full Matroska parser. But the matroska.py file looks like a full parser. I don't know how I can use this .py script in a user Lua-script, and I can't find this .py-script in my mpv folder. But if this script really working and accessible than it should indeed easy to parse Matroska file, and the way is open to code all the other features.

First the features should work with local files and later it can extend to internet files as well.

If you do develop a way to read mkv header information from inside Lua please let me know.

Yes, of course, but I think I need help to develop in Lua. I have download some Editors like Eclipse(no luck). How do you do developing Lua? I know Notpad++ can also be used, it has Highlighting but no debug feature. In an online Lua code editor was it not possible to run a Lua script because the "mp" object/declaration was not found.

Hidden and Enabled Flag - if you can get the information out of the mkv file about which chapters should be hidden then this is quite trivial, you simply remove the chapter from the chapter-list property during the on_preloaded hook

Yes for the hidden chapters it is the correct way to remove them from the chapter-list. But for disabled ordered chapters, the content have to be removed from the timeline: Better, it should not add to the virtual timeline. So I guess removing the disabled chapter from the chapter-list is not enough. But with the EDL system I think it should be easy to create a new own virtual timeline without the disabled ordered chapters. On the other hand this feature has the lowest priority, because I have never seen a disabled chapter. This is also more used by a Matroska Menu where a Chapter CODEC handles the content which have to play.

Nested Chapters - I don't entirely understand how much of this request is for the chapters to be read from the mkv metadata differently, .... If you want the chapters to be visually indented when displayed by mpv then you could probably just add extra whitespace to the start of the chapter title.

How this feature to implement is more or less clear to me. Read the meta data from the Matroska file and replace the chapters in the mpv chapter-list. Should be easy.

Nested Ordered Chapters - If you can read the mkv metadata yourself then you can manually create a virtual timeline however you want using the EDL specification. I think you can fully replicate ordered chapters using EDL but I'm not certain. You can disable mpv from doing the default ordered chapters using the --no-ordered-chapters option.

Yes this was also my first intention using the EDL sytsem. But I guess there are some test necessary to figure out if the --no-ordered-chapters option is important to use or to omit.

Native Matroska menu - .... but if you can figure out the timestamp or chapter marker when the jump should happen it is trivial to set a script to observe the related property change and run a seek command to move to a different chapter (or perform any other action).

Yes so easy it is. VLC observe the chapters when they are start end ends. I'm not sure but is there a property like "chapter-entry" or "chapter-change"? Which is the related property for that? Currently there is only one Native command GotoAndPlay(ChapterUID;) and this means only a jump to chapter. But this chapter is maybe part of an other Edition and a simple mpv-chapter-jump is not possible.

Content grouping - If you can figure out what the groups are, quite easy. Provide a script-message or script-binding that can be called by users to switch to a group. Call the relevant track switching commands. Very easy. Could even make a pop-up menu where you can select the group you want.

Very great. I had hoped this is so easy because changing a track or an Edition is already there. To change multiple track types is indeed a simple algo. Wow: pop-up-menu I had looked a bit, but found not really a good thing. I'm very sure if the time is coming I will use it, but also again I have to learn how it works.

Matroska DVD Menu - I don't know what these menus are supposed to be like, or if they consist of images/video components. But you can detect mouse clicks with keybinds and the mouse position with the mouse-pos property. You could probably insert the menu backgrounds with EDL or using the overlay-add to place images directly into the mpv window (see thumbfast). Doesn't sound worth it since it seems to be a dead feature, but I think it is theoretically possible to implement with a script.

I have no interest in this feature because it is outside of Matroska. A Chapter-CODEC has to know all the DVD-commands. But at the end the Matroska Interactive Menu(coming not soon) could use such things. It is like Netflix with all the buttons to control the playback -> skip intro, skip credits and so on. And it sounds interesting that a mouse-pos can be observed and images could be placed over the video. When the time is coming I need more info how this works.

If you make any attempts at implementing these features as one or multiple scripts feel free to ping me if you want any advice. I make no promises on the timeliness of my replies however.

Yes I will ping you, and no hurry. I also have a looong RL-Todo list and my chapterEditor needs an update to create multi-Edition-mkv's from UHD discs.

CogentRedTester commented 1 year ago

My first idea was to modify your script to parse a Matroska file by my self which eliminates the mkvinfo depence, because I fully understand the Matroska semantic and structure. I know it is not so easy to write a full Matroska parser. But the matroska.py file looks like a full parser. I don't know how I can use this .py script in a user Lua-script, and I can't find this .py-script in my mpv folder. But if this script really working and accessible than it should indeed easy to parse Matroska file, and the way is open to code all the other features.

Sorry I was misleading, that python script is not accessible from Lua. I just brought it up because I thought it might provide some inspiration for implementing a parser in Lua.

Yes, of course, but I think I need help to develop in Lua. I have download some Editors like Eclipse(no luck). How do you do developing Lua? I know Notpad++ can also be used, it has Highlighting but no debug feature. In an online Lua code editor was it not possible to run a Lua script because the "mp" object/declaration was not found.

I use Visual Studio Code with one of the Lua extensions. The mp module is provided by mpv and is only available when the script is run within mpv player. I don't know of any way of running and developing one of the scripts outside of mpv, so you had better get used to using print statements. The mp.msg module can make things a little nicer.

Yes so easy it is. VLC observe the chapters when they are start end ends. I'm not sure but is there a property like "chapter-entry" or "chapter-change"? Which is the related property for that? Currently there is only one Native command GotoAndPlay(ChapterUID;) and this means only a jump to chapter. But this chapter is maybe part of an other Edition and a simple mpv-chapter-jump is not possible.

You can observe the chapter property to detect when a chapter change happens. You can switch editions using the same method you switch tracks: setting the vid, aid, sid, and edition properties.

hubblec4 commented 1 year ago

Sorry I was misleading, that python script is not accessible from Lua. I just brought it up because I thought it might provide some inspiration for implementing a parser in Lua.

Ah OK, and no problem and yes I can use this as inspiration. With ChatGPT it is maybe easy to translate it to Lua.

I use Visual Studio Code with one of the Lua extensions.

Thanks I will try it.

The mp module is provided by mpv and is only available when the script is run within mpv player.

I had suspected that and it is clear to me that external debugging is not possible.

The mp.msg module can make things a little nicer.

Yeah I have read a bit already about this and saw it also in your script. I thought I can see all this info in mpv while running but I guess I have to activate something. Or have I to start mpv with params to show such mp.msg on screen?

You can observe the chapter property to detect when a chapter change happens.

Good to know. But when is this event fired? I guess when the new chapter is started. This would be not optimal, because Matroska offers an action for a chapter when it is finished. At this time is a next/new chapter not yet started. But we will see what's possible if it so far.

You can switch editions using the same method you switch tracks: setting the vid, aid, sid, and edition properties.

Perfect! I have some additions in my mind which are on top of Haali's TRACKSETEX maybe are possible.

hubblec4 commented 1 year ago

Hi @CogentRedTester

I wanted to inform you about my activities.

At first I looked at what I can do with the mpv (lib and exe). It also seems that it is not that difficult to use the libmpv directly in Lazarus. However, the effort to code a new player is much too big for me. The mpv.exe seems to be controllable from "outside", but apparently it is not really optimal.

So I decided to try the scripts after all.

With your tip for VS code, was very helpful and everything was set up quite quickly to code Lua.

I then took a closer look at the matroska.py script to see what was being done there. It was very easy to see that this is not a full Matroska reader. Not all elements are defined and important readout functions, such as standard values or 0-based size elements, are not available. The attribute Mandatory is also not covered.

Since I program with Lazarus and am very used to types and classes, it's not that easy for me to code a "simple" reader. That's why I encapsulated everything neatly in classes, since Lua apparently supports this sufficiently. Even simplified in some places, for example I was able to create a variable called "value" directly in the base class, which can then be used for all EBML types.

I have already translated the c++ libEBML and libMatroska to Lazarus, so it wasn't very difficult to transfer it to Lua. Since it is now only about reading a file, everything has become extremely simple.

Could you please take a look at the whole thing and tell me where and what I could still improve as far as the Lua syntax is concerned. Since this is my first Lua project, I'm sure I'll make a few mistakes. ebml.zip

Here is a small test script ebml_test.zip

Note that the project is not finished.

If you say that what I did there is good, then it's not difficult to do it for Matroska. A long time ago I had built myself a tool (EBMLObserver) with which I can examine EBML and all its derivatives more quickly and generate the necessary code. To do this, I built the export to Lua code into the tool.

CogentRedTester commented 1 year ago

Hi @hubblec4, I've had a look through your code. I don't know anything about EBML, so I can't comment on your design decisions or the accuracy of that part of the code, but overall it looks very good. However, I do have a number of suggested revisions.

Firstly a small point, I believe the remove_zero_bytes function can be significantly simplified using the string.gsub function:

local function remove_zero_bytes(str)
    return string.gsub(str, "%z", "")
end

Now onto the main issue, mpv supports three different versions of Lua; Lua 5.1, Lua 5.2, and LuaJit. Any individual build of mpv may support any one of these versions, so if you want the script to be universally compatible with mpv you need to make sure your code works with all three. This is not as hard as it sounds since Lua 5.2 makes very few breaking changes to Lua 5.1, and LuaJit is just running Lua 5.1 with a few of the additions from 5.2. Generally you should stick to only using features present in the Lua 5.1 Manual, but take a quick read of the incompatibilities section of the 5.2 manual every now and then to make sure you haven't stumbled onto one of the few breaking changes.

You have used two pieces of Lua syntax that are not compatible with the three versions of Lua. There may be others I have not noticed. Firstly is the goto statement. This is supported by Lua 5.2 and LuaJit, but not Lua 5.1. You should be able to easily replace your use of this statement by placing the code between the goto and the label into an else block. The second issue requires a little more work. Bitwise operators were added in Lua 5.3, they are not in any of the versions of Lua mpv supports. Lua 5.2 and LuaJit have inbuilt bit operation libraries, but Lua5.1 does not, so you may want to add your own bit operation functions and use those. This Stack Overflow page seems to have some decent examples.

hubblec4 commented 1 year ago

Many thanks for your reply and the suggestions.

Firstly a small point, I believe the remove_zero_bytes function can be significantly simplified

Yes, that's a great simplification. ChatGPT has generated this complicated function :-)

Now onto the main issue

Oh dear, that's really a strong limitation in terms of bit operations. As far as I understand, this only works for positive 4-byte numbers, but the integers that Matroska uses have 8-bytes and can also be negative.

But I found one thing very nice, and that was reading the floats. I was able to determine the floats directly using a native Lua function string.unpack(">d", buffer) I just checked what you can still output with string.unpack. Integers seem to work there too. With this you could reduce the whole read_data() functions extremely. The only question then would be which code is faster.

Of course, the best would be mpv implements support for Lua5.3. Would something like this take a long time, or is that not even planned?

The "goto" problem should be easily fixed though.

CogentRedTester commented 1 year ago

Oh dear, that's really a strong limitation in terms of bit operations. As far as I understand, this only works for positive 4-byte numbers, but the integers that Matroska uses have 8-bytes and can also be negative.

I don't have an answer for you since I don't know how Matroska uses those numbers.

But I found one thing very nice, and that was reading the floats. I was able to determine the floats directly using a native Lua function string.unpack(">d", buffer) I just checked what you can still output with string.unpack. Integers seem to work there too. With this you could reduce the whole read_data() functions extremely. The only question then would be which code is faster.

string.pack and string.unpack were also added in Lua 5.3, so you cannot use them either.

Of course, the best would be mpv implements support for Lua5.3. Would something like this take a long time, or is that not even planned?

See https://github.com/mpv-player/mpv/issues/5205, https://github.com/mpv-player/mpv/issues/9712, and the FAQ.

hubblec4 commented 1 year ago

Hi @CogentRedTester

I had a bit time to work on LuaEBML and there is now a GitHub repo.

It was a great journey through the world of bits and bytes. Programming without bit operations is a real challenge these days, so it would really make sense to consider updating to Lua5.3.

Anyway, I hope I managed to make everything fit for older Lua versions now.

So, the next step is to create the Matroska.lua library. I will initially only consider the meta elements. Parsing a Block element is a bit more involved, and I suspect we won't need it at all.

CogentRedTester commented 1 year ago

Hi @hubblec4, it appears to be working for me, I'm getting the same output as I was getting from the Lua 5.3 code. A couple of notes: I am getting an error from you nesting multiple [[ ]] pairs inside one another in the header. You can avoid this by using a level 1 long bracket ([=[ ]=]) for the outer brackets:

--[=[ Lua Verison compatiblity: 
    Lua 5.1, Lua 5.2 and LuaJIT are not able to handle Bit operations fully
    Lua 5.3 (and higher, 5.3++) has proper build-in Bit operation support  

    I use my own "Compiler switch" technic to support both cases directly
    Code for Lua5.3++ uses: --[[LuaNew <- start switch
    Code for older Lua versions uses: -- [[LuaOld <- start switch
    for switching you have to make a manual String replace
    Xxx = "New" or "Old"
    activate LuaXxx: change "--[[LuaXxx" to "-- [[LuaXxx"
    disable LuaXxx: change "-- [[LuaXxx" to "--[[LuaXxx"    

    default version is LuaOld
--]=]

You could also automatically detect which version Lua is running with using the value of _G._VERSION.

hubblec4 commented 1 year ago

Hi @CogentRedTester

Many thanks for the tips.

And further goes... Creating the Matroska lib was very easy, only few changes to get the base code. Lua seems to handle only 200 local var's, so I had to encapsulate all 259 elements. matroska.zip

This is a first start and some functions are missing, but it is now possible to parse a Matroska file. The Block and SimpleBlock element data are only read out but not parsed in the sense of the Matroska Block-Structure.

I have another question about the "require". Does the user then have to adapt it to his needs himself or is there a possibility of a general specification for integrating the ebml library.

I will make also a GitHub repo, when you say it is fine. I have startet to code a Matroska Parser which makes than all more easier.

I had in my mind such things like: MkParser = MkParser:new() MkParser.task = {Chapters, Tags, Info} MkParser.parse

EDIT: how can I make a code section like you in your posts? The code icon insert two chars but you see the result in my posts are looks different to your code sections.

CogentRedTester commented 1 year ago

This is a first start and some functions are missing, but it is now possible to parse a Matroska file. The Block and SimpleBlock element data are only read out but not parsed in the sense of the Matroska Block-Structure.

The code isn't crashing, but I don't see any way to give it a Matroska file to parse. Until we can give it a Matroska file and compare the output to another parser like mkvinfo I don't know how to check that it's working.

I have another question about the "require". Does the user then have to adapt it to his needs himself or is there a possibility of a general specification for integrating the ebml library.

mpv currently does not have a standard folder that scripts load modules from. I usually specify that my modules be placed in the ~~/script-modules/ directory. This requires that developers that make use of the library to add an additional line before the require statement:

package.path = mp.command_native({"expand-path", "~~/script-modules/?.lua;"})..package.path
local module = require 'module'

I will make also a GitHub repo, when you say it is fine. I have startet to code a Matroska Parser which makes than all more easier.

I had in my mind such things like: MkParser = MkParser:new() MkParser.task = {Chapters, Tags, Info} MkParser.parse

I don't know what the whole MkParser.task stuff means, but I figure we want to include some functions that are as simple as possible. Ideally I'd imagine you could do something like this:

local MKVParser = require 'matroska-parser'

local mkvEDML = MKVParser.parse('/path/to/file.mkv')

-- I don't know if these would be the actual names of the fields
local uid = mkvEDML.info.SegmentUUID

You could of course include less abstracted functions for more specific uses that I may not be aware of.

EDIT: how can I make a code section like you in your posts? The code icon insert two chars but you see the result in my posts are looks different to your code sections.

Use a block of triple ` characters as detailed here. You can also include the name of the programming language after the first set of backticks to do language-sensitive syntax highlighting:

```lua
local module = require 'module'
hubblec4 commented 1 year ago

The code isn't crashing, but I don't see any way to give it a Matroska file to parse. Until we can give it a Matroska file and compare the output to another parser like mkvinfo I don't know how to check that it's working.

No crashing sounds good. Yes this matroska.lua script is "only" a library with all the Matroska elements and some internal logic, like the c++ libMatroska. For parsing/reading a Matroska file there is more code necessary. I have make some progress on this also: I write a Matroska-Parser to make everything simple.

mpv currently does not have a standard folder that scripts load modules from.

I have figured it out a bit by my self and I had read the mpv specs for this. There is a method for using scripts which uses more than one .lua script. I had created a folder and placed all scripts there, and use a main.lua script, but it doesn't work.

For testing I use your segment-linking.lua script. I have changed everything so that no mkvinfo.exe is needed. But it will not work, and I'm sure I make some faults.

I don't know what the whole MkParser.task stuff means, but I figure we want to include some functions that are as simple as possible.....

Yes I can imagine that all stuff for you is not clear at the moment. I had also needed a long time to understand EBML and Matroska, but at the end it is simple, you will see. :-)

Yes the Matroska Parser gets methods to keep everything simple for a developer. For example to handle Hard-Linking: There are two methods for handling Hard-Linking.

-- Hard-Linking is used
function Matroska_Parser:hardlinking_is_used()
    -- check if the Hard-Linking is used
    -- retrun@1 : boolean - is used?
    -- return@2 SegmentUUID, return@3 PrevUUID, return@4 NextUUID

    -- WebM don't support Hard-Linking
    if self.is_webm then return false end

    -- check the UIDs
    local seg_id, prev_id, next_id

    seg_id = self.Info:find_child(mk.info.SegmentUUID)
    if seg_id then seg_id = seg_id.value end
    -- note: Hard-Linking will also work if there is no SegmentUUID

    prev_id = self.Info:find_child(mk.info.PrevUUID)
    if prev_id then prev_id = prev_id.value end

    next_id = self.Info:find_child(mk.info.NextUUID)
    if next_id then next_id = next_id.value end

    -- no prev or next UID -> no Hard-Linking
    if prev_id == nil and next_id == nil then return false end

    -- Ordered Chapters or Soft-Linking overrides Hard-Linking
    -- check the Chapters
    if self.Chapters == nil then
        -- if no chapters present -> Hard-Linking is used
        return true, seg_id, prev_id, next_id
    end

    -- TODO chapter parsing
    if #self.Chapters.value == 0 then
        self.Chapters:read_data(self.file)
    end

    -- get the default edition and check if ordered chapters are used
    local def_edition = self.Chapters:get_default_edition()
    if def_edition:get_child(mk.chapters.EditionFlagOrdered).value then
        if def_edition:find_child(mk.chapters.ChapterAtom) then
            return false
        end
        -- no chapter found -> Hard-Linking is used
    end

    -- all is checked and Hard-Linking is used
    return true, seg_id, prev_id, next_id
end

-- Hard-Linking get UIDs
function Matroska_Parser:hardlinking_get_uids()
    return self.Info:find_child(mk.info.SegmentUUID),
           self.Info:find_child(mk.info.NextUUID),
           self.Info:find_child(mk.info.PrevUUID)
end

Note: The UUID's are in binary format, and maybe it is necessary to change the format to a string with hex-values for the output to be working with your method "get_uids()".

I attach my test folder so that you can play a bit with the Matroska Parser. hardlinking.zip

EDIT: I see an issue with the function Matroska_Parser:hardlinking_get_uids(). It returns the elements and not the UUIDs.

hubblec4 commented 1 year ago

Hi @CogentRedTester

It's working :-) One issue was that the UIDs not returned as strings(hex-string) and a small issue with using the module correctly.

But now both variations working, your extra folder "script-modules" with the expanded package.path and with a folder and a main.lua direct in the scripts folder.

Here again my test folder. hardlinking.zip

For testing I use the Hard-Linking samples from my Matroska-Playback repo.

Now is the way open to do many things for Matroska.

I have in my mind to write a Matroska-Playback.lua script with all the Matroska features we want to have. This master script should also handle the entire logic for Hard-Linking (and later all other features) like file loading and folder scanning and so on.

CogentRedTester commented 1 year ago

Hi @hubblec4, sorry for the delay. Unfortunately, the script is not working for me, I am getting the following error message:

[hardlinking] stack traceback:
[hardlinking]   ...s\____\AppData\Roaming/mpv/scripts/hardlinking/ebml.lua:805: in function 'find_next_element'
[hardlinking]   ...pData\Roaming/mpv/scripts/hardlinking/matroskaparser.lua:99: in function '_validate'
[hardlinking]   ...pData\Roaming/mpv/scripts/hardlinking/matroskaparser.lua:53: in function 'new'
[hardlinking]   ...s\____\AppData\Roaming/mpv/scripts/hardlinking/main.lua:92: in function 'get_uids'
[hardlinking]   ...s\____\AppData\Roaming/mpv/scripts/hardlinking/main.lua:245: in function 'fn'
[hardlinking]   mp.defaults:597: in function 'handler'
[hardlinking]   mp.defaults:510: in function 'call_event_handlers'
[hardlinking]   mp.defaults:552: in function 'dispatch_events'
[hardlinking]   mp.defaults:503: in function <mp.defaults:502>
[hardlinking]   [C]: at 0x7ff79063cdc0
[hardlinking]   [C]: at 0x7ff7906397c0
[hardlinking] Lua error: ...s\____\AppData\Roaming/mpv/scripts/hardlinking/ebml.lua:674: attempt to index local 'parent_s' (a nil value)
client removed during hook handling
Sending hook command failed. Removing hook.

I tested this with kuchikirukia's Cardcaptor Sakura release, and with a set of test files I created with mkvtoolnix. You can find my set of test files here.

hubblec4 commented 1 year ago

Hi @CogentRedTester

Thanks for testing and pointing me out to an issue which is already known in Matroska.

The issue you found was in the find_next_element() function. The Segment element is found but all checks if this is a valid element fails, because the max_read_size was to small. (All my test files was smaller than 15mb) OK, in the Matroska Parser I could setup a hugh read_size or I use the entire file size, but this is not a good solution(IMO).

In the c++ libMatroska is additional a find_next_ID() function, but I remember me that Mosu had reported some issues with that. For a long time it was defined that the Segment element is directly after the EBML element. But there can be also Void elements and damaged data.

Therefore I had decide to use my own way to read root elements like all other elements only with the find_next_element() function. Currently are max 15mb parsed to find a Segment element.

The Matroska_Parser handles now the file loading. hardlinking.zip

Very interesting is, that MPC-HC don't play your Hard-Linking sample properly. MPC-HC starts, shows the correct duration, the cover is shown and the sound is there, but it is impossible to jump in the timeline, MPC-HC freeze.

mpv works fine now with your sample.

CogentRedTester commented 1 year ago

Hi @hubblec4, I can confirm that all my hard-linked files are now working. I have a couple of suggestions for the module.

Firstly I think it would be useful to have a stringify method on the EBML elements so that you can easily print an element and its children for debugging purposes. It also might be useful to be able to pass the Matroska_Parser a file handler instead of just a file path. What I am considering is how this might be used in conjunction with io.popen to read network files, perhaps in conjunction with wget. However I'm not actually certain that io.popen works with the read binary (rb) flag. Definitely something worth testing.

MPC-HC loads those files fine for me, I can seek no problem. You might be using an older version of MPC or LAV Filters.

hubblec4 commented 1 year ago

Firstly I think it would be useful to have a stringify method on the EBML elements so that you can easily print an element and its children for debugging purposes.

OK. Could you explain this a bit more in details what we need? I guess each element needs a "Name" field and than we can output this element name, right? A Master element has than a method to loop over all children.

It also might be useful to be able to pass the Matroska_Parser a file handler instead of just a file path.

Yes maybe this is a better way. I looked at your mpv-read-file script and I'm not quite sure how it handles Matroska files that are not available locally. The description says that it is intended for text files. I did a test on this where I access Matroska files over the network that use Hard-Linking. That worked without any problems with io.open().

The "rb" flag is maybe not necessary, in my tests it worked also without this flag. I use this flag because this was my first info after reading about io.open().

MPC-HC loads those files fine for me, I can seek no problem. You might be using an older version of MPC or LAV Filters.

OK, I have to check this.

CogentRedTester commented 1 year ago

OK. Could you explain this a bit more in details what we need? I guess each element needs a "Name" field and than we can output this element name, right? A Master element has than a method to loop over all children.

Think mkvinfo where it gives you a nice formatted output of an element and all it's children elements:

|+ Segment information
| + Timestamp scale: 1000000
| + Multiplexing application: libebml v1.3.5 + libmatroska v1.4.8
| + Writing application: mkvmerge v20.0.0 ('I Am The Sun') 64-bit
| + Duration: 00:23:40.480000000
| + Date: 2018-03-19 18:14:23 UTC
| + Segment UID: 0xbf 0x8c 0x8d 0x54 0x32 0x4e 0x88 0x21 0x96 0x2e 0x55 0x51 0x53 0x7c 0x34 0xc3
| + Previous segment UID: 0x72 0x77 0xc3 0x31 0xcf 0x62 0xe9 0x55 0xa6 0xd1 0x5d 0x79 0x8c 0x44 0x94 0x60

But it doesn't need to look like this, it could also look like a JSON string instead for example. Just some way to show an element and all it's children in a human readable purpose for debugging purposes. There is the mp.utils.to_string function, but this can be difficult to read for large values. It also doesn't work on your current version because you're storing everything in the metatable instead of the actual table. I believe this to be a bug in fact, the Matroska_Parser:new(path, do_analyze) function is using self for all its operations, which is causing operations to run on the global Matroska_Parser table instead of the new elem table you are making for each element. You can actually see this happen if you run two Matroska_Parser:new operations at once, they interfere with each other and the results of the second parse overwrite the first. Changing the function to this instead:

Matroska_Parser.__index = Matroska_Parser

-- Matroska Parser constructor
function Matroska_Parser:new(path, do_analyze)
    local elem = setmetatable({}, Matroska_Parser)
    elem.path = path
    elem.file = nil

    -- Validate
    elem.is_valid, elem.err_msg = elem:_validate()

    -- Analyze, find all Top-Level elements
    -- do_analyze = false: means the user have to analyze manually
    if elem.is_valid and (do_analyze == nil or do_analyze == true) then
        elem.is_valid, elem.err_msg = elem:_analyze()
    end

    -- close file if invalid
    if not elem.is_valid then elem:close() end
    return elem
end

fixes the issue and actually stores values in the Lua elem table properly.

I did a test on this where I access Matroska files over the network that use Hard-Linking. That worked without any problems with io.open().

The "rb" flag is maybe not necessary, in my tests it worked also without this flag. I use this flag because this was my first info after reading about io.open().

On my computer io.open does not work with files available over http and other network protocols (network mounted file systems are obviously fine). I'm talking about doing something like this:

function foo()
    local f  = io.popen('wget -q -O - "https://github.com/mpv-player/mpv/files/12286265/hardlinking.zip"', 'r')
    print(f:read('*l'))
    f:close()
end

foo()
hubblec4 commented 1 year ago

Think mkvinfo where it gives you a nice formatted output of an element and all it's children elements:

OK, yes I think that should not be so heavy. Is the output as JSON better than as Text? Should I add an option for both cases?

I believe this to be a bug in fact, the Matroska_Parser:new(path, do_analyze) function is using self for all its operations

Oh yes of course, I had the same issue while creating EBML and Matroska Lua libs. The concept of classes in Lua is not entire clear to me, so I have some question:

Matroska_Parser.__index = Matroska_Parser

-- Matroska Parser constructor
function Matroska_Parser:new(path, do_analyze)
    local elem = setmetatable({}, Matroska_Parser)

Why is Matroska_Parser.__index = Matroska_Parser not within the constructor function? Is this line of code equal to self.__index = self which is coded currently in all my classes?

-- Matroska Parser constructor
function Matroska_Parser:new(path, do_analyze)
    local elem = setmetatable({}, self)
    self.__index = self

Is this class construct also correct?

On my computer io.open does not work with files available over http and other network protocols (network mounted file systems are obviously fine). I'm talking about doing something like this:

OK, in fact I haven't tested that. But you have more experience with that and I trust you. I had thought about it:

CogentRedTester commented 1 year ago

Why is Matroska_Parser.__index = Matroska_Parser not within the constructor function?

Because the Matroska_Parser table is being used as the metatable for every new parser object. That means the __index field only needs to be added once. If you put that line into the constructor, then you are re-assigning the index field to the same value every time you call Matroska_Parser:new(). It doesn't cause any issues, but I personally find that setting the __index field outside the constructor makes it easier to understand that the same table is being re-used multiple times. Obviously it is your choice, however.

Is this line of code equal to self.__index = self which is coded currently in all my classes?

Only when you call Matroska_Parser:new() directly. However, because the new method is also part of the Matroska_Parser table, it means the method is made available to parser objects as well, meaning you can do something like this:

local mkv = Matroska_Parser:new('path')
local mkv2 = mkv:new('path2')

In the second instance self would refer to the mkv parser table, not the Matroska_Parser table, which will cause the new parser to inherit values from the previous parser which is... probably not your intention? For this reason you may also want to change function Matroska_Parser:new(path, do_analyze) to function Matroska_Parser.new(path, do_analyze), to better reflect the fact that the constructor method does not require any prior state and is only designed to return a standard parser object. Note however that if you do intend for mkv:new() operations to inherit values then self.__index = self and function Matroska_Parser:new(path, do_analyze) is correct. You can also just mandate that people shouldn't be using the library like that which is probably also fine. Programming with Lua uses your approach, but it is doing so with the intention of implementing inheritance.

  • It would be really good that all file loading is handled in the Parser
  • the Parser can also handle network protocols - maybe using your mpv-read-file script or as native implementation

That's up to you.

  • it is maybe not a problem to support both cases as input for the constructor

Yes I think you should support both.

hubblec4 commented 1 year ago

Hi @CogentRedTester

Why is Matroska_Parser.__index = Matroska_Parser not within the constructor function?

Because the Matroska_Parser table is being used as the metatable for every new parser object. That means the __index field only needs to be added once.

OK, that sounds logic and I have tested it, but this code produces many issues:

ebml_element.__index = ebml_element
-- EBML element construtor
function ebml_element:new()
    local elem = {}
    setmetatable(elem, self)    
    return elem
end

Also when I change only this one line, I get a lot of warnings and errors:

-- EBML element construtor
function ebml_element:new()
    local elem = setmetatable({}, self)    
    self.__index = self
    return elem
end

In the second instance self would refer to the mkv parser table, not the Matroska_Parser table, which will cause the new parser to inherit values from the previous parser which is... probably not your intention?

I create all classes with the intention that someone can use them as a base class so I hope everything is OK with my class constructors.

Here now the news: In the EBML and Matroska c++ lib's there is no to_string() method for the elements, so I didn't implement that either. But therefore the Matroska_Parser now has a elem_to_string(elem, verbose) method. Every element can be passed to this method: (don't do it for a large Segment element)

    local mkp = Matroska_Parser:new(path)
    print(mkp:elem_to_string(mkp.Info, true))

I tried to emulate the output of mkvinfo. But it's not the same when it comes to the names of the elements. The element names are used directly, which are also human-readable.

This was a good addition because I found some issues which are now fixed and I could easy implement other Matroska features -> Matroska Ticks and Segment Ticks. For the Track Ticks I have to check how I could implement this.

Yes I think you should support both.

And again the Parser can use a loaded file as input. The path is still necessary as input.

LuaEBML LuaMatroska LuaMatroskaParser

hubblec4 commented 1 year ago

Hi @CogentRedTester

I've been working a little further in the last few days and there is now a mpvMatroska script(folder). But mpvMatroska is still very fresh and has a lot of construction sites and certainly still errors.

The aim was to get some simple Matroska features up and running.

mpvMatroska handles Hard-Linking now perfectly and a bit better than MPC-HC(crashs on Endless-Loop test). Unfortunately, your segment linking script has a weak point that I know of, which VLC and other players also have. Linking up the files is not correct because mpv uses the Info/Duration as the playing time. However, the duration of the video track must be used, which is slightly shorter in 99.9% of cases. The error can be recognized by the fact that the chapter markers no longer match the correct frame.

mpvMatroska currently only gets the video track duration from the statistics tags that MKVToolNix writes. The own reading of the video duration comes later.

There are already some chapter features built in. The editions and chapters are processed completely by themselves and transferred to mpv.

Nested chapters are already displayed correctly in mpv but there are problems "navigating" to a chapter. When chapters have the same times, mpv doesn't seem to fit at all.

Nested-Ordered-Chapters somehow already works, but I don't quite understand how and what mpv is doing in the background. The test file from my Matroska-Playback repo is loaded properly and 70 seconds of playing time are also displayed and the chapters are correct shown, but after 30 seconds the playback is over. The timeline jumps to the end and the video ends. Likewise, you cannot reach a timestamp greater than 30 seconds in the timeline.

Linked-Chapters are already handled well by mpv itself, so I'm not entirely sure whether mpvMatroska takes care of everything or whether mpv is still involved.

But there seems to be problems with the editions. mpv itself always gives me the name of the previous edition after I have changed an edition.

Furthermore, no editions of "Only-Linked-Chapters.mkv" files are apparently processed. instead, the editions that mpv finds in the first linked file are taken.

My Mainmenu.mkv files often have 20 or more editions but mpv only shows me 5 editions.

Added a new description for the EditionDisplay elements in the Matroska-Playback repo. However, when I pass my edition list to mpv, no names are used. It may be that I'm doing something wrong or mpv has a bug there.

In the mpv log I always see that the chapters are being parsed, even if the edl_path contains "!no_chapters". I guess this only refers to the output of the chapters for the timeline.

Is there a way to stop mpv from parsing the chapters? Because it seems to me that mpv as soon as there are Matroska features are hands on.

Furthermore, it is not really clear to me how the setting of the editions is supposed to work at all. Just because I can send mpv a list doesn't mean that mpv also knows the virtual timeline that is connected to it.

So far my approach is to oversee the changing of an edition. mpv restarts the playback anyway, and at this point I wanted to load the corresponding new virtual timeline.

hubblec4 commented 1 year ago

Is there a way to stop mpv from parsing the chapters? Because it seems to me that mpv as soon as there are Matroska features are hands on.

I can now answer this myself. I have disabled mpv's ordered-chapters option and now works the Nested-Ordered-Chapters feature perfectly(except chapter jumping with the OSC), also the Linked-Chapters with a SegmentUUID works. That's so great, because now it is easy to support Linked-Editions

CogentRedTester commented 1 year ago

I create all classes with the intention that someone can use them as a base class so I hope everything is OK with my class constructors.

Yes in that case ignore what I said.

Here now the news: In the EBML and Matroska c++ lib's there is no to_string() method for the elements, so I didn't implement that either. But therefore the Matroska_Parser now has a elem_to_string(elem, verbose) method. Every element can be passed to this method: (don't do it for a large Segment element)

Looks mostly good for me, but two issues:

Firstly if I try to print the top-level element directly:

local mkp = lmp.Matroska_Parser:new(path)
print(mkp:elem_to_string(mkp, true))

I get an error: [hardlinking] Lua error: ...pData\Roaming/mpv/scripts/hardlinking/matroskaparser.lua:379: attempt to call method 'is_master' (a nil value)

Second I'm getting some weird output for some of the numbers, for example:

|+ Info (data position: 4157, data size: 145)
...
| + Duration: -9223372036854775808:00:00.000000000 (data position: 4248, data size: 4)

vs mkvinfo:

|+ Segment information
...
| + Duration: 00:23:40.480000000

And again the Parser can use a loaded file as input. The path is still necessary as input.

This works, but it may be simpler to do something like this:

-- Matroska Parser constructor
function Matroska_Parser:new(file, do_analyze)
    local elem = setmetatable({}, self)
    self.__index = self

    -- if file is a string then treat it as a path, otherwise treat as a file object
    if type(file) == 'string' then elem.path = file
    else elem.file = file end

    -- rest of the function
end

Unfortunately, your segment linking script has a weak point that I know of, which VLC and other players also have. Linking up the files is not correct because mpv uses the Info/Duration as the playing time. However, the duration of the video track must be used, which is slightly shorter in 99.9% of cases. The error can be recognized by the fact that the chapter markers no longer match the correct frame.

Interesting, I will look into this.

However, when I pass my edition list to mpv, no names are used. It may be that I'm doing something wrong or mpv has a bug there.

Furthermore, it is not really clear to me how the setting of the editions is supposed to work at all. Just because I can send mpv a list doesn't mean that mpv also knows the virtual timeline that is connected to it.

Yes, I don't think you can set manual editions, I do not believe mpv provides any access to the internal data that the edition timelines are based on. You may be able to implement the logic yourself, but I'm not sure how that would interact with mpv's internal logic.

As for the rest of your comments, you are starting to go beyond what I can help you with. I lack the required knowledge of mpv's internals and the intended behaviour of the various Matroska features you are trying to implement. I'm still happy to help you out, but if so you'll need to provide me with more specific instructions about which files to play and the exact behaviour that should be happening when mpv plays the file. That way I'll actually be tell if a feature is working or not, because currently I'm running some of your test files with your latest script and I have no idea.

hubblec4 commented 1 year ago

Hi and many thanks for your time to test my work.

Firstly if I try to print the top-level element directly:

In your code sample you try to stringify the entire mkp object, and that is not possible.

local mkp = lmp.Matroska_Parser:new(path)
print(mkp:elem_to_string(mkp, true)) --< wrong
-- you must pick an element, all Top-Level elements are there:
print(mkp:elem_to_string(mkp.Info, true))
print(mkp:elem_to_string(mkp.Chapters, true))
print(mkp:elem_to_string(mkp.Chapters:find_child(mk.chapters.EditionEntry), true))
--... and so on

Second I'm getting some weird output for some of the numbers, for example:

Mmmh, not good...I hope I find the issue

This works, but it may be simpler to do something like this:

This will not really work, because the file path is needed!! When I pass only a loaded file object to Matroska_Parser:new(), I guess I can't figure out the file path later? Without the file path I can't create the edl_path timeline.

I know loading network files and other not local things are important, for you and many others. At the beginning I will focus to get working the Matroska features for local files. If all works I think it should be easy to support loading files from network (I hope).

Yes, I don't think you can set manual editions, I do not believe mpv provides any access to the internal data that the edition timelines are based on. You may be able to implement the logic yourself, but I'm not sure how that would interact with mpv's internal logic.

I had afraid that. It's good that I've already considered that and take care of the respective timelines of each edition myself. I still have to find out how and where exactly I have to integrate this into mpv. But after reading for some time, I already think that there are too few options that mpv provides to be controlled.

.... I lack the required knowledge of mpv's internals and the intended behaviour of the various Matroska features you are trying to implement. I'm still happy to help you out ...

Ok, I will try to explain in future more in details. I'm a bit overwhelmed myself right now. Everything is new territory for me too and I feel like a boy who has been given a train. :-) There's a huge difference between just developing the specs and getting the functions up and running yourself. It's great fun right now.

For me the best feature are the nested-ordered-chapters: The test file has 60 seconds video content. The result playing time will be 70 seconds. The chapters are constructed to skip some parts of the video and some parts are doubled. The intention for nested-ordered chapters is to combine video content of multiple files and presented in a nested chapter structure to get/see quickly the correct chapter.

hubblec4 commented 1 year ago

Hi @CogentRedTester

You could help me much with sharing your test file for the Duration-issue(weird timestamp). If the file is very big you could cut the first 3mb, this should be enough to get the header elements. But you must cut the file with a binary splitter like my hsplit. A cutted file with MKVToolNix changes the headers.

hubblec4 commented 1 year ago

Hi @CogentRedTester @ghost @mathstuf

When I wanted to add the Linked-Edition chapters, I noticed that my algorithm wasn't able to do this. So I looked at what mpv had already done and found this PR. https://github.com/mpv-player/mpv/pull/258 But this feature is not working in mpv.

I have re-written the core-code of mpvMatroska to get a heuristic which creates for each Edition a virtual chapter_timeline. From this chapter_timeline it is easy to create the EDL path and the mpv chapters list. Additionally, this chapter_timeline will be useful for all chapter processes (Matroska menu).

I don't want to bore you any further with Matroska's specialist knowledge. I wrote some text and uploaded test files in my Matroska Playback repo. Maybe it is helpful for this issue. https://github.com/mpv-player/random-stuff/issues/5

However, the following feature might be of interest: Multiple chapter names.

I added a support for that. mpvMatroska change the chapter names automatically while observing the audio id. When the audio language match a chapter names langauge the new name is used for the mpv chapter list. If no audio language match, then the first chapter name is used.

The language from a selected subtitle track is taken into account on creating the mpv chapter list first time. I plan to add to observe the subtitle id also to change the chapter names automatically.

I like this feature, because I have some series with multiple audio and chapter languages, and now I can see all of them.

There is also new text and test files for this feature.

You can download here a new test version

Since all chapter types now work, it won't be long until features are added to intercept user interactions and graphical displays.

At the moment I still see a problem when handling the editions. mpv definitely creates an incorrect list when linked segments come into play.

If the edition is changed with the short cut "Shift+E", then I would have to intercept that, change the edition myself, and mpv is not allowed to change the edition itself. Do you have an idea?

Likewise, the display for the chapters isn't necessarily super stylish, and you can't click on any chapters either. I found a script that makes this possible, but I would like to have my own solution.

Furthermore, all players lack an edition selection button so that you can quickly change the edition. In the LAV splitter tray icon is such a nice Popup menu, but not in the player.

An extra pop-up menu would also be required for content grouping feature.

Is there a way to monitor clicking in the OSC? Changing chapters doesn't work if there are chapters with the same times.

CogentRedTester commented 1 year ago

Hi @CogentRedTester

You could help me much with sharing your test file for the Duration-issue(weird timestamp). If the file is very big you could cut the first 3mb, this should be enough to get the header elements. But you must cut the file with a binary splitter like my hsplit. A cutted file with MKVToolNix changes the headers.

I am using the files here: https://drive.google.com/drive/folders/17ZnEs7tYsRZY4cWtaQ_7l2ZzJTdbeiCd. I created these files myself using MKVToolNix.

There is some weird behaviour. If I look at the first two files of the 10-part split (made with the 'split before chapter' option) with MediaInfo you can see that the chapter being split appears in both files:

Menu
00:00:00.000                : 1. Yoru no Sei/夜の精
00:04:31.000                : 2. Fuyu no Chikadou/冬の地下道
Menu
00:04:32.279                : 2. Fuyu no Chikadou/冬の地下道
00:09:59.000                : 3. Oumagatoki/逢魔ヶ時

Now this is partially to be expected, it is part of the behaviour of MKVToolNix to add a new chapter marker to the start of the second file if the start of the file falls within the bounds of the chapter that ended the previous file. My segment-linking script is designed to handle this by merging adjacent chapters with the same name. We can see this behaviour in another split of the same file into larger segments (10-part) that don't align with the chapter markers:

Menu
00:00:00.000                : 1. Yoru no Sei/夜の精
00:04:31.000                : 2. Fuyu no Chikadou/冬の地下道
00:09:59.000                : 3. Oumagatoki/逢魔ヶ時
Menu
00:10:03.276                : 3. Oumagatoki/逢魔ヶ時
00:15:54.000                : 4. Koishita Hito e/恋した人へ
00:19:40.000                : 5. Tsubaki wa Ochitakaya/椿は落ちたかや

And for reference this is the original file's chapters:

Menu
00:00:00.000                : 1. Yoru no Sei/夜の精
00:04:31.000                : 2. Fuyu no Chikadou/冬の地下道
00:09:59.000                : 3. Oumagatoki/逢魔ヶ時
00:15:54.000                : 4. Koishita Hito e/恋した人へ
00:19:40.000                : 5. Tsubaki wa Ochitakaya/椿は落ちたかや
00:24:20.000                : 6. Yoidore Shibai/酔ひどれ芝居
00:30:06.000                : 7. Akai Hana/紅い花
00:33:48.000                : 8. Hatachi ni Nareba/二十才になれば
00:37:24.000                : 9. Yukionna/雪女
00:45:33.000                : 10. Mihatenu Yume/見果てぬ夢

You can see that the chapter times are all accurate if we take the earlier chapter time as the correct one, so the chapters are accurate to the original file.

However, there are some weird discrepancies, the first chapter finishes at 00:04:31.000, and yet the first file is 00:04:32.279 long. Now you may think that this is just the split occurring at slightly the wrong spot. However, if you compare the duration of the output files as reported by mpv then you get slightly different results:

original file: 2898.061s 5-part segment file: 2898.407s 10-part segment-file: 2898.852s

So the same file is actually getting longer as the number of segments increases. However, what noticeable effect is this actually having on the output file? Well I decided to go a little further with my testing. I split the same file into 543 segments. After doing so the full timeline when loaded by mpv had a total duration of 2946.836s, approximately 48 seconds longer.

However, here is the interesting thing, the chapter times were also shifted to compensate, the final chapter of the original video was at the approximate mpv timestamp 00:45:33, while the final chapter of the 543 segment timeline was at 00:46:18. As a result the chapter switch happens at the same time relative to the video and audio. I did notice the occasional audio break while playing the file though, so there are gaps in the timeline. I tested with a video file, and you can tell there is a slight stutter in the video and the audio when the segment changes over. I've never noticed before because the videos I've watched before have always had a segment break at the same time as a scene break, so there usually wasn't any video or audio to stutter. VLC seems to just crash at the stutter point, but that may be something to do with the duplicate chapter markers instead (or maybe something unique to those files?), I do have another copy of the 5-part files without any chapter markers and vlc does play the file (albeit with a larger stutter than mpv).

So is this a problem with mpv? Or is it a problem with my files? Well given that both mpv and VLC show the extended timeline duration, and the fact that the extra duration of the segment is shown in MediaInfo as well, I would say it's probably a problem with the files and/or MKVToolNix.

So what does this mean for our scripts? Well since mpv is handling the chapter offsets, the only issue is the slight stutter that is noticeable only if the file has a segment split in the middle of a scene. Whether we want to do anything about this, and if we even can do anything, is unclear.

TLDR

Split mkv files seem to have a slightly longer duration than the actual content of the file, potentially caused by MKVToolNix. Usually this is unnoticeable because of how small the additions are, but when using a sufficiently large number of segments (10s or 100s) it can actually add 10s of seconds to the timeline. However, mpv automatically offsets the chapter marks and the missing segment of the timeline causes only a very minor stutter to the video and audio that is only noticeable if it occurs in the middle of a scene.

hubblec4 commented 1 year ago

Hi @CogentRedTester

Many thanks for sharing your test files, I will download the files and then I can fix the timestamp issue.

identical chapter names

Yes I have seen this in your script. There is a function to delete such doubled chapters. And I'm not sure if this is a good behaviour of MKVToolNix(MTX) to set the same chapter info again in the next splitted file.

I will look into this to remove/replace such chapters also.

Hard-Linked files

I will try to explain what's going on and why VLC, mpv and many other players have issues to play such linked files. Only MPC-HC and my mpvMatroska handles that correctly.

Matroska has only one Duration element which is ever the longest duration of all streams in the file. For technical reasons (Codec design, Matroska design) there are some audio frames behind the last video frame.

For example your "Moon Chips-002.mkv" (times in nano seconds)

Video duration: 603269000000
Segment Duration:   603365000000

You can see the Segment Duration is 96ms longer. Almost all players uses the Segment Duration and the result is a video gap on the transitions.

+++video+data+++gap---next-video-data--- +++audio+data+++++---next-audio-data-----

This gap is the additional time which will be added for each splitted file. An unsplitted file has only one times such a video gap at the end of the file.

VLC seems to just crash at the stutter point, but that may be something to do with the duplicate chapter markers instead (or maybe something unique to those files?),

No it is a VLC bug. Many Hard-Linked files worked to play but with stutter/hanging video on the transitions.

the chapter times were also shifted to compensate, the final chapter of the original video was at the approximate mpv timestamp 00:45:33, while the final chapter of the 543 segment timeline was at 00:46:18. As a result the chapter switch happens at the same time relative to the video and audio.

Yes the chapter times are shifted but still wrong to show the correct video frame.

So is this a problem with mpv? Or is it a problem with my files?

This is definitiv a mpv issue. Your files are fine. On the other hand is Hard-Linking not the best solution to link content. You/all should always use ordered chapters to control which content you want to play. Nothing is more precise than ordered chapters, and much player support this kind of chapter.

So what does this mean for our scripts?

For mpvMatroska is everything working, but I'm afraid you will not be able to fix this in your Segment-Linking script.

You have to create a better EDL path: Only the filename as input is wrong, because mpv uses then the Segment Duration. You must set the video duration as playtime length and a 0 for the start time. But I don't know if mkvinfo can report the duration of a video stream. There is also no element in Matroska to store a video duration. MTX writes statisitcs Tags with the video duration info. mpvMatroska parse this Tags and use from there the video duration, for the moment. MTX stats Tags are not always present and I have to implement to read the Clusters to find the last video frame.

local duration = file:get_video_duration() or file.seg_duration

If a file has no video stream then is the Segment Duration fine BUT not perfect, because:

What is when you link an audio file which have also a subtitle stream and the subtitle stream has a duration of 1h, but the audio duration is only 3min. When we use the Segment Duration then is there a audio gap of 57min.

TLDR

What means TLDR?

mpv should parse Matroska files better, because never trust the Segment Duration.

Hrxn commented 1 year ago

What means TLDR?

It means TL;DR

. . . . . .

Too Long;Didn't Read

hubblec4 commented 1 year ago

Hi all

Here is a new test version of mpvMatroska. I have implemented handling multiple editions and multiple edition names.

mpvMatroska overrides the short-cut "E" to change/toggle the editions. mpvMatroska now observes also the subtitle change and change the edition and chapter names if the subtitle language match.

@CogentRedTester I have try to use your mpv-scroll-list script but with out luck. Also the chapter-list script don't work for me. What have I to do?

magnilens commented 11 months ago

Hi @hubblec4, I was interested in making some feature requests and a search suggested you may already have something similar to what I was interested in.

Like you, I'm interested in making Hidden Chapters not appearing in the timeline or chapter selection lists. How's that one coming along?

The other thing I'm interested in is the displayed title for ordered chapter videos. At present, MPV uses the title of the first segment of video to be played as the displayed title for the whole ordered video. Most of the time, that isn't what I want. Instead, I have the title I prefer in the mkv with the ordered chapters list. MPC-HC would use this as the title for the full video. I would love it if MPV could be made to do the same. Do you have anything that could do that?

hubblec4 commented 11 months ago

Hi @magnilens

Like you, I'm interested in making Hidden Chapters not appearing in the timeline or chapter selection lists. How's that one coming along?

Yes, mpvMatroska don't show hidden chapters in the timeline and also not in a selection list.

The other thing I'm interested in is the displayed title for ordered chapter videos.........

I'm not fully sure to understand. A title has nothing to do with ordered chapters.

An MKV has a field "Title", do you mean this element value? Multiple Editions can be inside an mkv and the names/titles of the editions are currently not correct handled by mpv, but mpvMatroska is working.

magnilens commented 11 months ago

An MKV has a field "Title", do you mean this element value?

Yes, that. As an example, I have the ordered chapters in one mkv, call it Index; and the actual video referred to by the chapters in another video, call this one Show. To make a neat presentation, I would give Index a proper title in its the Title field. In MPC-HC, the Title I set for Index would be displayed when the file is played. But in MPV, it would be the Title for Show that would be displayed instead. If the ordered chapters in Index referred to multiple different mkvs, like Show1, Show2, Show3, etc., the Title of the first mkv (Show1 here) stays displayed for the entire video, even when MPV moves on to segments from the other mkvs.

I would like to have the Title for Index, in this example, displayed when the file is opened for playback, regardless of whether anything in the file is played or not.

hubblec4 commented 11 months ago

Ahhh now is it clear to me.

You use ordered-linked-chapters to play linked content from other mkv's. Very nice, I use also such Index.mkv's for my series collections.

And yes mpv uses the title from the linked mkv's and also the Editions from the first linked mkv, which is definitive an issue.

In the current state of mpvMatroska, the file name of the input/main file (in your case the Index.mkv) is used, but not the value from the Title field, and additional the current Edition name is appended. For the test phase you can download in the first post the latest version. https://gleitz.info/forum/index.php?thread/48553-mpvmatroska/&postID=466696#post466696

But it should be easy to uses the Title field value instead of the file name.

I had make some progress last night and "uosc" is now used when the user had installed it. uosc is a very nice GUI and the Editions selection works now perfect. Also Matroska Content-Grouping feature is ready. I will make a new test version this or next night.

magnilens commented 11 months ago

For the test phase you can download in the first post the latest version.

Yes, hidden chapters are now invisible! I would like to know, can these scripts be used with other mpv-derived players? I'm mostly using mpv.net actually.

Update: Found out where to put the scripts in mpv.net and they work there too! mpv.net does have an issue in that the player doesn't load the tracks info from the linked files, so only the tracks in the input file are shown on their track selection menu. At least they're still selectable from the OSC. But that's not a problem with your scripts, so I'm not sure if there's anything you can do about it.

But it should be easy to uses the Title field value instead of the file name.

Look forward to it. Until then, having the filename of the main/input file displayed does 80% of what I want.

uosc is a very nice GUI and the Editions selection works now perfect.

Sounds interesting, where can I find this uosc?

hubblec4 commented 11 months ago

I would like to know, can these scripts be used with other mpv-derived players?

Yes me too, and I think it should work, because this kind of scripting is a native mpv thing. Nice to read that mpv.net works.

mpv.net does have an issue in that the player doesn't load the tracks info from the linked files,.....

First a question: Do you use tracks inside the Index.mkv? If yes then mpv does the correct job. You should use a "only-linked-chapters.mkv" without any tracks. mpv now uses the correct tracks from the linked files.

Sounds interesting, where can I find this uosc?

https://github.com/tomasklaen/uosc

magnilens commented 11 months ago

Do you use tracks inside the Index.mkv?

While tracks exist inside the index.mkv file, they are not linked by the ordered chapters at all. It's just a one-second video to provide a thumbnail for the file.

mpv now uses the correct tracks from the linked files.

It certainly does. Mpv.net just doesn't pick that up in its right-click menu. But, like I said, those tracks are still selectable via the OSC. That's not your problem. I'll bring it up with them.

https://github.com/tomasklaen/uosc

Yes, I found them already. They have practically all the features I'm using mpv.net for. If I can figure out how to get the preview window and right click menu enabled, I might just move back to mpv from mpv.net.

And thanks for the scripts, it's great, even if I can only make limited use of it right now.

hubblec4 commented 11 months ago

A new test version is uploaded and much more info is provided in GitHub.

Now it is possible to open selectable menus with uosc.

hubblec4 commented 11 months ago

@magnilens I have provided a new Test version Now the file Title (if present) is used for mpv's media-title.

magnilens commented 11 months ago

It works! Thank you so much for making these scripts.

Now to get on to figuring out the features I want to enable in uosc.

magnilens commented 11 months ago

I wanted to try out multiple editions in a single mkv. But, from what I can tell with my own mkv files and your own mkv playback test files, mpvMatroska and uosc is not fully capable of processing the additional editions beyond the default one at the moment. Is that the case, or did I configure something wrong?

Right now, the edition menu in uosc doesn't show the custom edition names. Those custom names and the chapters listing in the editions aren't updated in the title bar and chapter menu respectively when I change editions using the edition menu. Also, editions are accessible even if I marked them as Hidden.

I also had an mkv with ordered chapters linked to the mkv with multiple chapters. When I opened the ordered chapters for playback, uosc showed the multiple editions from the linked mkv, even though the ordered chapters had only one edition. The chapters menu showed the chapters from the ordered chapter, not the linked mkv though.

hubblec4 commented 11 months ago

Yes this is a big issue of mpv. uosc uses only the info from mpv and shows "wrong" lists of chapters and editions.

On mpvMatroska GitHub start page I explain how you can set up an extra edition list, handled by mpvMatroska.

magnilens commented 11 months ago

Thank you, I managed to figure out the button. I suggest you mention that users need to replace the <has_many_edition>editions entry on the controls= line in uosc.conf with <has_many_edition>command:bookmarks:script-binding mpvMatroska/open-editions?Editions I also added #editions>1 before the ? to have the badge showing the number of editions.

Speaking of which, if I used linked chapters, the edition button would appear with a badge showing "2" for the number of editions in the file being played, not for the number of editions in the input file, which has only 1. Yet, both menu and hotkey are only able to access the edition(s) in the input file and can't switch editions.

I see that Editions marked as Hidden are still shown by the button and selectable by the menu and hotkey.

And what's with the long string of numbers after the edition name in the edition menu? Is it related to the edition playing time?

hubblec4 commented 11 months ago

Hi @magnilens

I suggest you mention that users need to replace.....

Thanks, and yes you are right, I will add this info.

command:bookmarks:script-binding mpvMatroska/open-editions?Editions I also added #editions>1 before the ? to have the badge showing the number of editions.

has_many_editions and also #editions>1 uses the info from mpv which is not ever correct. I have to figure out how I can use own variables to count the editions so that the badge are correct. (And I don't know if I can use own variables for the count of badges.)

I see that Editions marked as Hidden are still shown by the button and selectable by the menu and hotkey.

I will check this, and I think I forgot to remove such editions from the list.

And what's with the long string of numbers after the edition name in the edition menu? Is it related to the edition playing time?

Yes it is the playing time of an edition.

magnilens commented 11 months ago

'has_many_editions' and also '#editions>1' uses the info from mpv which is not ever correct.

With all the issues I'm learning about with respect to editions, I think I will forgo using them for now. I originally wanted to use an extra hidden edition to hold some information for use in encoding the source video, in case I ever wanted to reencode it. But now, I think I can hide that information in hidden chapters and the comments I see your chapterEditor can put in chapters? Or not. Comments got wiped out when the chapters were edited by another program that didn't recognize them.

Yes it is the playing time of an edition.

The number started with a minus sign and was perhaps a dozen digits long before "00:00" at the end. I think it might have overflowed.

hubblec4 commented 11 months ago

With all the issues I'm learning about with respect to editions

Yes it is a shame that all the players don't handle editions well or correct. Currently I have a lot of work with my cE, but I will continue on mpvMatroska.

The best way to preserve such info, you should use Matroska Tags for that. A Tags editor is also on board with cE.

I see your chapterEditor can put in chapters?

This is only a bonus feature in cE and can't preserve if you save chapters to an mkv, this is only for storing in .xml files.

The number started with a minus sign...

Mmmmh not good. I have to check this

hubblec4 commented 11 months ago

For anyone interested in the new Matroska content grouping feature, there is a test version of cE here. Editing and creating content groups is now done with just a few clicks.

aicynide commented 10 months ago

Upvote