Plugin descriptor improvements, part 1
- Capability string constants.
- Disabling of unnecessary option cat entries for efficiency.
- Plugin-wide reorganisation of plugin descriptor content for better clarity, consistency, readability, and reducing mistakes.
- Renaming of plugin descriptor begin/end macros to reflect that the descriptor blocks are really plugin descriptors, that wrap content that includes one or more module descriptions, avoiding confusion for newbies. (They wrap plugin property setters (rare), option descriptors (which are held by the plugin not modules), and one or more module descriptions, and thus it is cofusing and wrong for them to be called 'module' begin/end wrappers).
- Option string review part 1 - ditching pointless duplicated descriptions causing pointless tooltips.
- A small handful of little fixes.
A lot of time and effort went into this big chunk of work. Please be kind. There is a some more that builds on top of this to come later.
I know that this is a lot. Please do not think that I do not value the time of reviewers, I do. I feel apprehensive at how you're going to feel about reviewing the big commits in this. The big stuff is all rather straightforward though; I've checked it over several times since producing it and after rebases and cherry-picking for this MR, and will no doubt check once again for any stray mistake after a merge, so by all means just skim through it and check that you're happy in principle if you want. I cringe at the idea of you all spending hours combing through it all and getting annoyed at me, or worse rejecting it as not worth your time. Please don't.
Further large bits of completed work building upon this to submit later includes:
- Fixing a lot of mistakes with names, descriptions, shortcuts, etc.
- Further big option string review covering other common issues.
- Getting rid of
set_category()
(having interfaces look them up from the subcats). - Lots of option bounds checking enhancements.
- Type-strict plugin macros.
- Possible merging of
set_capability()
andset_callbacks()
as part of an incomplete type-strict (cap specific) activate/deactivate call interface. - Rework of how config data is passed to the plugin descriptor handler.
- Previously discussed enum based capability work.
Merge request reports
Activity
- Resolved by Lyndon Brown
Hi,
except in a few rare cases, plugins without options do not need to make calls to set cat/subcat, but many do, which is pointless inefficiency, so let's address that by commenting out such unnecessary calls.
I'm sorry, I'm not sure to understand why, can you elaborate?
added MRStatus::Reviewable label
- Resolved by Lyndon Brown
Hello,
Previously discussed enum based capability work.
We already decided in the ML that we were not in favor of using define for module capabilities.
The main reason is to be able to create submodules type without having to modify VLCCore.
added MRStatus::InReview label and removed MRStatus::Reviewable label
added MRStatus::NotCompliant label and removed MRStatus::InReview label
added 79 commits
-
1fb3e0d0...183c8d99 - 56 commits from branch
videolan:master
- c5ab854c - disable unnecessary plugin option set cat calls
- 3e9160d2 - reorganise plugin descriptors
- 98ceb7f1 - partial option string review
- f80658c6 - cdda+vcd: fix confusing help text output
- ffeb7cfd - plugins: purge now unused `add_usage_hint()`
- 340ed5c9 - mp4: fix help output bug
- 82603792 - plugins: privatise `add_category_hint()`
- d209f907 - x264: don't use `vlc_config_set()` directly
- 913e58c9 - rename vlc_module_[begin|end] to vlc_plugin_[begin|end]
- 4bf4306a - add defines for module capabilities
- bfdd0cc4 - core: use true/false not 0/1 for bool options
- 962a93ab - rename MODULE_NAME_IS_* to PLUGIN_NAME_IS_*
- 617185f1 - i420_yuy2: fix double translation
- 1c380db0 - medialibrary: fix double-translation
- b4668c51 - asf: purge long unused constant
- 4d74bec9 - qt: fix option text typo
- b0e2780b - sapi: fix missing option text translation
- c0bfdb57 - aom: fix missing option text translation
- bfb57cf5 - mmal: fix missing option text translation
- 0ffd89d1 - vaapi: fix missing option text translation
- 8df1610b - rav1e: fix missing option text translation
- a00c8b67 - opengl: fix missing option text translation
- d03b493e - core: add missing translation for --extractor-flatten label
Toggle commit list-
1fb3e0d0...183c8d99 - 56 commits from branch
added MRStatus::InReview label and removed MRStatus::NotCompliant label
mentioned in merge request !12 (merged)
Okay. I do understand; with the length of review processes, tons of old work I'm eager to get merged, and not being certain how much people would be happy to cope with at once, finding the right balance in cases like this is a work in progress.
I would argue that it's all about improving plugin descriptors though (with the cap constants just also helping elsewhere). I'm perfectly happy to break this submission up into smaller chunks if that makes people happy and gets things moving, I just need a little guidance on where people want it split. :)
The big/huge ones here are:
- 1 "disable unnecessary plugin option set cat calls"
- 2 "reorganise plugin descriptors"
- 3 "partial option string review"
- 9 "rename vlc_module_[begin|end] to vlc_plugin_[begin|end]"
- 10 "add defines for module capabilities"
The rest are small.
Where should I break this set at for this MR? Just the first? First 2, 3?
I can't really split out # 10 to come first though, that would be a huge amount of extra work.
Edited by Lyndon BrownEach should be a MR and likely not submitted at the same time if they depend on each other. I'd go with the least problematic to the most problematic. And yes, it's hard to predict in advance what will cause problems and what will not. Maybe start with smaller things as well, while we're all going through the transition to Gitlab.
I'm in the process of splitting it up, with the smaller items already moved out. Once there's some movement on those I'll start getting the remaining big ones done one by one.
I may as well close this in the meantime at least since it's not going to make any progress as it is.
Maybe start with smaller things as well, while we're all going through the transition to Gitlab.
Trying to start with smaller things isn't easy. Events on the ML some time ago drove me to go off and work on my own for some time, producing one big branch with hundreds of commits, with these big ones near the start. (Little got processed after I returned, and I'm trying to take this opportunity to get things moving again). Some stuff is easy to extract randomly, as I have been doing, whilst a lot else is not. I have little choice if I want things to get moving than to publish stuff from the start of the branch, but I am trying to publish smaller stuff as well. The hope of getting this chunk out of the way in one go clearly was too much.
mentioned in merge request !49 (merged)
mentioned in merge request !89 (merged)
added 39 commits
-
d03b493e...b731340c - 33 commits from branch
videolan:master
- 6cd74e90 - disable unnecessary plugin option set cat calls
- 43ec6958 - reorganise plugin descriptors
- 645da11c - partial option string review
- c8ca53c1 - rename vlc_module_[begin|end] to vlc_plugin_[begin|end]
- 1b3a810d - add defines for module capabilities
- 21460b24 - rename MODULE_NAME_IS_* to PLUGIN_NAME_IS_*
Toggle commit list-
d03b493e...b731340c - 33 commits from branch
mentioned in merge request !90 (merged)
mentioned in merge request !96 (merged)
added MRStatus::NotCompliant label and removed MRStatus::InReview label
mentioned in merge request !113 (closed)
added MRStatus::Duplicate label and removed MRStatus::NotCompliant label
mentioned in merge request !805 (merged)