[streamdetails] Add SubtitleCodec and SubtitleType #25023
Conversation
enen92
added
Type: Feature
| @@ -86,6 +86,8 @@ class CStreamDetailSubtitle final : public CStreamDetail | |||
| void Serialize(CVariant& value) const override; | |||
| bool IsWorseThan(const CStreamDetail &that) const override; | |||
|
|
|||
| std::string m_codec; | |||
| std::string m_type; | |||
There was a problem hiding this comment.
Name: "type" is a generic word, maybe "location" or "storage" or "storage type" is better?
audio tracks could use that field as well.
type; non-localized strings "muxed" or "external" can work for storage but are not great for display to the user and I don't think translation should be up to the skin.
I don't know if there could ever be more values than internal/external > a bool could be enough. Or a number for an enum.
There was a problem hiding this comment.
Subtitle Type cannot be a boolean because this property will be used to know if there is or not a subtitle - and if you set it to false, what would that mean ? no subtitle or external one?. Codec cannot be used for this, because it is only retrieved if the subtitle is internal.
To my point of view, Type property is fine (why not an enum), better than location (which sounds more like network or local values).
|
I like the idea and have been wanting to see the subs codec in the skin (same way the screen opened by o shows video and audio codec and info). |
| const CStreamDetailSubtitle* item = | ||
| dynamic_cast<const CStreamDetailSubtitle*>(GetNthStream(CStreamDetail::SUBTITLE, idx)); | ||
| if (item) | ||
| return item->m_type; |
There was a problem hiding this comment.
| return item->m_type; | |
| return item->m_codec; |
And the reverse in the function below?
Description
This is yet another alternative to #25000 and adds the codec type (if available) for subtitles as well as their type (muxed/external). This allows skins to rely on properties that should always be defined (i.e. the type) instead of the language as the single source of truth.
Motivation and context
Currently to check if a file has subtitles skins check for the subtitle language property. However:
If no subtitle:
String.IsEmpty(ListItem.Property(SubtitleLanguage.1) = true
If there is a subtitle but language that isn't defined:
String.IsEmpty(ListItem.Property(SubtitleLanguage.1) = true
With this change we have the information more consistent with audio and have also properties that should always be defined indirectly solving the issue above.
How has this been tested?
Runtime tested
What is the effect on users?
Skins can use the subtitle type property to check if a given subtitle exists.
Screenshots (if appropriate):
Types of change