search

Matroska-Org / libmatroska

a C++ libary to parse Matroska files (.mkv and .mka)
GNU Lesser General Public License v2.1
318 stars 57 forks source link

rename functions & classes with `Timecode` in their names #137

Closed mbunkus closed 8 months ago

mbunkus commented 8 months ago

For historical reasons there are several classes & functions that use the word timecode in their name, whereas what they're dealing with are actually timestamps, not timecodes. When working on the specs we decided to rename the elements in the specs but not in the libraries for compatibility purposes. As we're breaking the API & ABI left & right already, maybe now's the time to fix this inconsistency in the libraries, too?

robUx4 commented 8 months ago

Yes. I stumbled upon some timecode names in the code and thought the same thing.

I think we should rename the old methods, and maybe keeping the old names as redirections marked as deprecated. Or we can just fully switch and get rid of the old names.

It seems keeping compatibility between old and new libs (ie on Linux depending on the distro) will require some ifdef, so having more won't hurt too much.

mbunkus commented 8 months ago

You definitely need some #ifdefs already if you want to support v1.x & v2.0 simultaneously. For me it's even "worse" as roughly 1/12 of all of my test cases now have different checksums with v2 due to one of changes we made (don't remember which one exactly; it's definitely fine, just requires some more manual handling).

Therefore I'm for simply getting rid of the old names (just like we just got rid of DefaultISset(), for example).