[addons][audiodecoder] rework about audiodecoder addon system #20317
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlwinEsch
added
Type: Feature
AlwinEsch
force-pushed
the
cleanup-audiodecoder-system
branch
from
78d7b39
to
dd7e88d
Compare
This was referenced Oct 15, 2021
AlwinEsch
force-pushed
the
cleanup-audiodecoder-system
branch
from
dd7e88d
to
b5fea6e
Compare
This was referenced Oct 16, 2021
AlwinEsch
force-pushed
the
cleanup-audiodecoder-system
branch
from
b5fea6e
to
2adc884
Compare
This was referenced Oct 16, 2021
AlwinEsch
force-pushed
the
cleanup-audiodecoder-system
branch
from
5bab650
to
e08cd64
Compare
…qual to C This change has to keep the function names of the "C" part the same as the static am on the C++, only "ADDON_" remains as the beginning, because I suspected that with e.g. "init" there could be problems in other places. Another reason is the planned future automation of the interface between Kodi and addon, where it will be automatically added to the C ++ header. Hence, names must be kept the same.
This structure was previously unused and also comes into conflict with planned future ones, where it becomes more difficult to use it in all circumstances. In addition, the creation of the new instance is a bit trickier because of the executed EXE type and where there would then be a problem with the structure so that the structure is to be transferred before the recipient is even there and ensures that it runs. Therefore, such types are only transferred there when the said instance is running. Instead, requested values or the structure should be called up using direct functions.
The background is that in a sandbox system it is no longer possible to simply exchange a structure defined as a pointer between Kodi and addon, since Kodi never edits and uses this, it should always be passed as "void*" for something like that. In addition, it becomes smaller because the target handle / class pointer add-on is then transferred directly and no longer the current API base structure.
As far as possible, it must be possible to set up the header purely on "C" -based parent headers. In addition, that it will be removed in the upcoming request to AddonBase.h (where currently provided), the background is because no throws are used there and can therefore be gotten cleaner.
…nd rename struct In this commit, the structure AUDIO_DECODER_INFO_TAG is renamed once to KODI_ADDON_AUDIODECODER_INFO_TAG. The background is that for me a "_" is preferred for group division, and can also be used as a division of paths. So that a "C" structure roughly corresponds to a C ++ namespace. Furthermore, the more primary change in it, whereby the fixed array size is removed from strings and re-created using "strdup". This then has the following advantages: - The string length is irrelevant and does not need to be taken into account by e.g. lyrics - Storage space is saved because unnecessary areas of the fixed memory do not have to be set - It allows easier handling in upcoming sandbox basis systems
…cleanup namespace note
This uses the new macro ATTR_DLL_LOCAL that was previously introduced. Once with a change to CInstanceAudioDecoder and a new addition to AudioDecoderInfoTag, where it was missing.
This improves the value for transferring the channel list supported by the add-on at the init call of the audio decoder. Once you save the memory in Kodi's class and furthermore I had problems managing the "const enum AudioEngineChannel ** info" correctly when setting up "C" and its tests, because it refers to a fixed array, works with the new value now.
… to array. This is a mandatory guideline on the revised API! It must never be used as such `uint8_t* buffer, int size`, since the subsequent value size cannot be assigned to buffer with "int", so an auto-creation records this as a single value referring to the external location. In order to recognize this correctly, it must always be set with a size_t after its value (never before).
The background to this is that this value is required in several places in the Sandbox API and is easier to manage using typedef. It also makes the structure clearer to identify available functions. I left the _V1 in there for the time being, because it would comb anyway and to leave out subsequent changes. These commits and requests are mainly about having the finale cleaner and only recognizing the changes that are directly related to it.
With this change, a new add-on function is added to the audio decoder interface. This means that the add-on can be used to query whether a given file can be used in it. As default, if addon does not use this new function, it is "true". This makes the use of the function optional and not mandatory. The background to the use of this is as an example audiodecoder.sacd which uses *.iso as file extensions, unfortunately this is also used for many other things such as DVD, so this function confirms that the add-on can really process it. In order to keep this as small and standardized as possible, the new class KODI::ADDONS::IAddonSupportCheck has been added, which can also be used with other types. Unfortunately, since each type has to handle it in its processing class, each respective type has to create itself. In order to have e.g. CAddonDll there could be problems if new types are added and every add-on identifies itself as a processor (default true). **At "CFileExtensionProvider::CanOperateExtension" there is still something to do, hence the big warning and notes there.**
This changes the add-on function CInstanceAudioDecoder::ReadPCM to allow a new enum AUDIODECODER_READ_RETURN as return value. Thought to make it easier to understand for external developers. With integrity beforehand, it could easily be misunderstood as a loaded quantity.
This adds a cache class to manage the file extensions and mimetypes supported by audio decoders. In addition, this makes it possible to read a list of the formats supported therein and to display it as an example in a dialog (planned in subsequent commits). Furthermore, adding ".***stream" to addon.xml as a file extension to manage tracks is no longer applicable, as it is automatically inserted into it if tracks are supported by the addon. It will probably make sense in the future to expand this new class and make it available globally for all addons that act on file extensions (with shifting to another place). Bringing it in here, however, would exceed the volume of the request.
This is an important change to avoid possible conflicts with other file extensions. The name "stream" was previously too general and may cause problems if something else uses the same thing. In addition, its name is now as a define in addon header in order to be able to find used places in Kodi more easily and to keep it smaller when changes are made.
…k number This adds a new static function in the C ++ header of the audio decoder addon. This is intended to read the track number desired by Kodi from the called path. This is added here because actually all addons that support tracks use exactly the same code, so now in the header to avoid there.
This adds cover art support to the audio decoder interface. There are two ways of doing this, as with Kodi, the transfer of a memory with loaded image content and a way where only its path to Kodi is transferred. This is good for addons that only have them in stock as a file, and also good when the sandbox system comes in to avoid unnecessary shared mem copies.
…ecoder addons Furthermore, oggstream has been removed from MusicInfoTagLoaderFactory.cpp, as it is not really used by ogg files and since it was moved to external audio decoder add-ons it has never been used (since the associated code is no longer available).
With the newly planned sandbox / API level system it becomes necessary to have the "C" files matching its namespace. As an example, a "C" function in audiodecoder must use "kodi_addons_audiodecoder_some_function ()" as the name, with which the "_" acts as a namespace divider to begin with, if now, as before, it would see "audio" and "decoder" as separate areas. Advantageous for the automatic creation script, otherwise problems can arise for him.
This introduces support for displaying the supported formats and file extensions in the add-on dialog for audio decoder add-ons. In the addon window, this option is now allowed with the first button and shows the dialog when clicked. Currently there is only a pure representation in it, but should also be expanded in the future, so that the user can choose which add-on is used for what or switch it off completely. It will also be expanded in the near future so that it can be used with other types (audioencoder, image decoder, vfs and game addons).
This revises the way an audio decoder communicates its supported formats. Previously this was in a single line with "|" as a separator. If the list were to become very large, as is currently the case with the new support for text and image paths, it will no longer be transparent. Therefore it is now stored in its own XML group. In addition, two new icons are introduced into Kodi which are intended to represent the type.
This cleans up the used namespace of the two files using the new style which should now be used. In future, all of them in the "xbmc/addons" folder should be converted to this.
In connection with previous commits.
AlwinEsch
force-pushed
the
cleanup-audiodecoder-system
branch
from
e08cd64
to
d2749bf
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In connection with my sandbox / API level project, some independent things that are necessary for this type of work are introduced here.
Furthermore, some improvements have been integrated so that more is supported, such as cover art.
There is also a new dialog so that the supported formats can be displayed in the addon information dialog (will be expanded to include other addon types in the future).
It would be a bit too thick to list the contents of the individual commits above, hence all commits with a description in them.
A few commits here, which do not belong directly to audiodecoder, are also brought into other requests.
Motivation and context
To make it even more usable.
How has this been tested?
What is the effect on users?
Screenshots (if appropriate):
Types of change
Checklist: