home
products
contribute
download
documentation
forum
Home
Forums
New posts
Search forums
What's new
New posts
All posts
Latest activity
Members
Registered members
Current visitors
Donate
Log in
Register
What's new
Search
Search
Search titles only
By:
New posts
Search forums
Search titles only
By:
Menu
Log in
Register
Navigation
Install the app
Install
More options
Contact us
Close Menu
Forums
MediaPortal 1
Quality Assurance
Bugreports
MatroskaTagInfo uses the wrong casing for tags
Contact us
RSS
JavaScript is disabled. For a better experience, please enable JavaScript in your browser before proceeding.
You are using an out of date browser. It may not display this or other websites correctly.
You should upgrade or use an
alternative browser
.
Reply to thread
Message
<blockquote data-quote="doskabouter" data-source="post: 1264895" data-attributes="member: 98267"><p>Ah crap.</p><p>Just noticed there are three places where that xml is read...</p><p><a href="https://github.com/MediaPortal/MediaPortal-1/blob/659396cf777032aa77f58a131ad089706e3e82c1/mediaportal/WindowPlugins/GUIVideos/MatroskaTagLib.cs#L58" target="_blank">MediaPortal/MediaPortal-1</a></p><p>internal in MediaPortal.GUI.Video namespace</p><p>added a really long (>10 years) ago</p><p>and </p><p><a href="https://github.com/MediaPortal/MediaPortal-1/blob/a8cbb8a8e74d06d4dc86ac04af25ce83842d6b85/mediaportal/Databases/Video/IMDBMovie.cs#L67" target="_blank">MediaPortal/MediaPortal-1</a></p><p>public in MediaPortal.Video.Database namespace</p><p>added about 7 years ago</p><p>and</p><p><a href="https://github.com/MediaPortal/MediaPortal-1/blob/master/TvEngine3/TVLibrary/TVDatabase/TvBusinessLayer/MatroskaTagLib.cs#L148" target="_blank">MediaPortal/MediaPortal-1</a></p><p>public in TvDatabase namespace</p><p>added a really long (>10 years) ago</p><p></p><p>Codewise its better to remove the one in the IMDBMovie file, because I think its not at its proper place, but that one is public, so possibly used by other plugins...</p><p>The one in MatroskaTagLib is nowhere used in mediaportal, and because it's internal, there's no worries about other dependencies failing.</p><p></p><p>Pff. and another thing I noticed: MatroskaTagHandler.Persist is never called from mediaportal at least (being a public method, theoretically it could be called from somewhere else). So at least in recent mediaportals there is never going to be an xml file in the old format.</p><p></p><p>However, the only place where I found that such a xml file is written is in the tvservice, for recorded tv.</p><p>That's one area I don't like to change because of lots more dependencies (MP2 for example), but it gives me a good place to test!</p><p></p><p>So my solution is (as is committed now to my pullrequest <a href="https://github.com/MediaPortal/MediaPortal-1/pull/182" target="_blank">Bug mp1 4977 matroska tag info uses the wrong casing for tags by doskabouter · Pull Request #182 · MediaPortal/MediaPortal-1</a>):</p><p>removed the MatroskaTagLib.cs from mediaportal solution</p><p>removed persist code from mediaportal</p><p>removed unused properties from IMDBMovie's MatroskaTagInfo</p><p>added support for correct and incorrect xml-files.</p><p></p><p>So from my point of view this is mergeable without any regressions.</p></blockquote><p></p>
[QUOTE="doskabouter, post: 1264895, member: 98267"] Ah crap. Just noticed there are three places where that xml is read... [URL="https://github.com/MediaPortal/MediaPortal-1/blob/659396cf777032aa77f58a131ad089706e3e82c1/mediaportal/WindowPlugins/GUIVideos/MatroskaTagLib.cs#L58"]MediaPortal/MediaPortal-1[/URL] internal in MediaPortal.GUI.Video namespace added a really long (>10 years) ago and [URL="https://github.com/MediaPortal/MediaPortal-1/blob/a8cbb8a8e74d06d4dc86ac04af25ce83842d6b85/mediaportal/Databases/Video/IMDBMovie.cs#L67"]MediaPortal/MediaPortal-1[/URL] public in MediaPortal.Video.Database namespace added about 7 years ago and [URL="https://github.com/MediaPortal/MediaPortal-1/blob/master/TvEngine3/TVLibrary/TVDatabase/TvBusinessLayer/MatroskaTagLib.cs#L148"]MediaPortal/MediaPortal-1[/URL] public in TvDatabase namespace added a really long (>10 years) ago Codewise its better to remove the one in the IMDBMovie file, because I think its not at its proper place, but that one is public, so possibly used by other plugins... The one in MatroskaTagLib is nowhere used in mediaportal, and because it's internal, there's no worries about other dependencies failing. Pff. and another thing I noticed: MatroskaTagHandler.Persist is never called from mediaportal at least (being a public method, theoretically it could be called from somewhere else). So at least in recent mediaportals there is never going to be an xml file in the old format. However, the only place where I found that such a xml file is written is in the tvservice, for recorded tv. That's one area I don't like to change because of lots more dependencies (MP2 for example), but it gives me a good place to test! So my solution is (as is committed now to my pullrequest [URL="https://github.com/MediaPortal/MediaPortal-1/pull/182"]Bug mp1 4977 matroska tag info uses the wrong casing for tags by doskabouter · Pull Request #182 · MediaPortal/MediaPortal-1[/URL]): removed the MatroskaTagLib.cs from mediaportal solution removed persist code from mediaportal removed unused properties from IMDBMovie's MatroskaTagInfo added support for correct and incorrect xml-files. So from my point of view this is mergeable without any regressions. [/QUOTE]
Insert quotes…
Verification
Post reply
Forums
MediaPortal 1
Quality Assurance
Bugreports
MatroskaTagInfo uses the wrong casing for tags
Contact us
RSS
Top
Bottom