Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[streamdetails] Add SubtitleCodec and SubtitleType #25023

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[streamdetails] Add SubtitleCodec and SubtitleType #25023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions xbmc/cores/VideoPlayer/DVDFileInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,8 @@ bool CDVDFileInfo::DemuxerToStreamDetails(const std::shared_ptr<CDVDInputStream>
{
CStreamDetailSubtitle *p = new CStreamDetailSubtitle();
p->m_strLanguage = stream->language;
p->m_codec = pDemux->GetStreamCodecName(stream->demuxerId, stream->uniqueId);
p->m_type = "muxed";
details.AddStream(p);
retVal = true;
}
Expand Down Expand Up @@ -476,6 +478,8 @@ bool CDVDFileInfo::AddExternalSubtitleToDetails(const std::string &path, CStream
CStreamDetailSubtitle *dsub = new CStreamDetailSubtitle();
std::string lang = stream->language;
dsub->m_strLanguage = g_LangCodeExpander.ConvertToISO6392B(lang);
dsub->m_codec = v.GetStreamCodecName(stream->demuxerId, stream->uniqueId);
dsub->m_type = "external";
details.AddStream(dsub);
}
return true;
Expand All @@ -490,6 +494,7 @@ bool CDVDFileInfo::AddExternalSubtitleToDetails(const std::string &path, CStream
CStreamDetailSubtitle *dsub = new CStreamDetailSubtitle();
ExternalStreamInfo info = CUtil::GetExternalStreamDetailsFromFilename(path, filename);
dsub->m_strLanguage = g_LangCodeExpander.ConvertToISO6392B(info.language);
dsub->m_type = "external";
details.AddStream(dsub);

return true;
Expand Down
24 changes: 24 additions & 0 deletions xbmc/utils/StreamDetails.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,21 @@ void CStreamDetailSubtitle::Archive(CArchive& ar)
if (ar.IsStoring())
{
ar << m_strLanguage;
ar << m_codec;
ar << m_type;
}
else
{
ar >> m_strLanguage;
ar >> m_codec;
ar >> m_type;
}
}
void CStreamDetailSubtitle::Serialize(CVariant& value) const
{
value["language"] = m_strLanguage;
value["codec"] = m_codec;
value["type"] = m_type;
}

bool CStreamDetailSubtitle::IsWorseThan(const CStreamDetail &that) const
Expand Down Expand Up @@ -484,6 +490,24 @@ std::string CStreamDetails::GetSubtitleLanguage(int idx) const
return "";
}

std::string CStreamDetails::GetSubtitleCodec(int idx) const
{
const CStreamDetailSubtitle* item =
dynamic_cast<const CStreamDetailSubtitle*>(GetNthStream(CStreamDetail::SUBTITLE, idx));
if (item)
return item->m_type;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return item->m_type;
return item->m_codec;

And the reverse in the function below?

return {};
}

std::string CStreamDetails::GetSubtitleType(int idx) const
{
const CStreamDetailSubtitle* item =
dynamic_cast<const CStreamDetailSubtitle*>(GetNthStream(CStreamDetail::SUBTITLE, idx));
if (item)
return item->m_codec;
return {};
}

void CStreamDetails::Archive(CArchive& ar)
{
if (ar.IsStoring())
Expand Down
4 changes: 4 additions & 0 deletions xbmc/utils/StreamDetails.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor

@CrystalP CrystalP Apr 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

@dcfou dcfou Apr 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link

@dcfou dcfou Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The building has missed, any idea why ? I don't have any authorization to look at it.
This would be great if this PR will go ahead ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'll get back to this one once I free up my queue :')

std::string m_strLanguage;
};

Expand Down Expand Up @@ -124,6 +126,8 @@ class CStreamDetails final : public IArchivable, public ISerializable
int GetAudioChannels(int idx = 0) const;

std::string GetSubtitleLanguage(int idx = 0) const;
std::string GetSubtitleCodec(int idx = 0) const;
std::string GetSubtitleType(int idx = 0) const;

void AddStream(CStreamDetail *item);
void Reset(void);
Expand Down
30 changes: 20 additions & 10 deletions xbmc/video/VideoDatabase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,11 @@ void CVideoDatabase::CreateTables()

CLog::Log(LOGINFO, "create streaminfo table");
m_pDS->exec("CREATE TABLE streamdetails (idFile integer, iStreamType integer, "
"strVideoCodec text, fVideoAspect float, iVideoWidth integer, iVideoHeight integer, "
"strAudioCodec text, iAudioChannels integer, strAudioLanguage text, "
"strSubtitleLanguage text, iVideoDuration integer, strStereoMode text, strVideoLanguage text, "
"strHdrType text)");
"strVideoCodec text, fVideoAspect float, iVideoWidth integer, iVideoHeight integer, "
"strAudioCodec text, iAudioChannels integer, strAudioLanguage text, "
"strSubtitleLanguage text, iVideoDuration integer, strStereoMode text, "
"strVideoLanguage text, "
"strHdrType text, strSubtitleCodec text, strSubtitleType text)");

CLog::Log(LOGINFO, "create sets table");
m_pDS->exec("CREATE TABLE sets ( idSet integer primary key, strSet text, strOverview text)");
Expand Down Expand Up @@ -3249,11 +3250,12 @@ void CVideoDatabase::SetStreamDetailsForFileId(const CStreamDetails& details, in
}
for (int i=1; i<=details.GetSubtitleStreamCount(); i++)
{
m_pDS->exec(PrepareSQL("INSERT INTO streamdetails "
"(idFile, iStreamType, strSubtitleLanguage) "
"VALUES (%i,%i,'%s')",
idFile, (int)CStreamDetail::SUBTITLE,
details.GetSubtitleLanguage(i).c_str()));
m_pDS->exec(PrepareSQL(
"INSERT INTO streamdetails "
"(idFile, iStreamType, strSubtitleLanguage, strSubtitleCodec, strSubtitleType) "
"VALUES (%i,%i,'%s','%s','%s')",
idFile, static_cast<int>(CStreamDetail::SUBTITLE), details.GetSubtitleLanguage(i).c_str(),
details.GetSubtitleCodec(i).c_str(), details.GetSubtitleType(i).c_str()));
}

// update the runtime information, if empty
Expand Down Expand Up @@ -4237,6 +4239,8 @@ bool CVideoDatabase::GetStreamDetails(CVideoInfoTag& tag) const
{
CStreamDetailSubtitle *p = new CStreamDetailSubtitle();
p->m_strLanguage = pDS->fv(9).get_asString();
p->m_codec = pDS->fv(14).get_asString();
p->m_type = pDS->fv(15).get_asString();
details.AddStream(p);
retVal = true;
break;
Expand Down Expand Up @@ -6374,11 +6378,17 @@ void CVideoDatabase::UpdateTables(int iVersion)
}
m_pDS->close();
}

if (iVersion < 132)
{
m_pDS->exec("ALTER TABLE streamdetails ADD strSubtitleCodec text");
m_pDS->exec("ALTER TABLE streamdetails ADD strSubtitleType text");
}
}

int CVideoDatabase::GetSchemaVersion() const
{
return 131;
return 132;
}

bool CVideoDatabase::LookupByFolders(const std::string &path, bool shows)
Expand Down
2 changes: 2 additions & 0 deletions xbmc/video/VideoThumbLoader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,8 @@ void CVideoThumbLoader::DetectAndAddMissingItemData(CFileItem &item)
{
std::string index = std::to_string(i);
item.SetProperty("SubtitleLanguage." + index, details.GetSubtitleLanguage(i).c_str());
item.SetProperty("SubtitleType." + index, details.GetSubtitleType(i));
item.SetProperty("SubtitleCodec." + index, details.GetSubtitleCodec(i));
}
}

Expand Down