From 95ec90a4cb530d311fbb9faf9efb74082caa1dbc Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Wed, 29 Jan 2014 20:40:10 +0100 Subject: [PATCH 1/4] Revert "[Fix][DVD/OMXPlayer] This fixes a regression introduced in 50a1d3c." This reverts commits bb1aeb75e13adc0bdb6befb69feca44997e5552b and fdacd42bcebc4197c75a24c6ea54511b796b396e. The fix is invalid and causes a mismatch between GUI assumed subtitle and what the player actually is playing. --- xbmc/cores/dvdplayer/DVDPlayer.cpp | 4 ---- xbmc/cores/omxplayer/OMXPlayer.cpp | 4 ---- 2 files changed, 8 deletions(-) diff --git a/xbmc/cores/dvdplayer/DVDPlayer.cpp b/xbmc/cores/dvdplayer/DVDPlayer.cpp index a32628ad92068..7f6681b7dc18d 100644 --- a/xbmc/cores/dvdplayer/DVDPlayer.cpp +++ b/xbmc/cores/dvdplayer/DVDPlayer.cpp @@ -832,11 +832,7 @@ void CDVDPlayer::OpenDefaultStreams(bool reset) { CloseSubtitleStream(true); if (m_pInputStream && !(m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD) || m_pInputStream->IsStreamType(DVDSTREAM_TYPE_BLURAY))) - { SetSubtitleVisible(false); - if (GetSubtitleCount() > 0 && CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream == -1) - CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream = 0; - } } // open teletext stream diff --git a/xbmc/cores/omxplayer/OMXPlayer.cpp b/xbmc/cores/omxplayer/OMXPlayer.cpp index 58f32d18264c7..db00d77196930 100644 --- a/xbmc/cores/omxplayer/OMXPlayer.cpp +++ b/xbmc/cores/omxplayer/OMXPlayer.cpp @@ -888,11 +888,7 @@ void COMXPlayer::OpenDefaultStreams(bool reset) { CloseSubtitleStream(true); if (m_pInputStream && !(m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD) || m_pInputStream->IsStreamType(DVDSTREAM_TYPE_BLURAY))) - { SetSubtitleVisible(false); - if (GetSubtitleCount() > 0 && CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream == -1) - CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream = 0; - } } // open teletext stream From 1288e581cddf67add57c7b4c0d423857ea324ed6 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 9 Feb 2014 20:41:44 +0100 Subject: [PATCH 2/4] dvd/omxplayer: add internal version of SetSubtitleVisible This avoids overriding result of OpenDefaultStreams forced/relevant calculation due to SetSubtitleVisible being delayed by message queue. Original-patch-by: ace20022 --- xbmc/cores/dvdplayer/DVDPlayer.cpp | 27 ++++++++++++++++----------- xbmc/cores/dvdplayer/DVDPlayer.h | 1 + xbmc/cores/omxplayer/OMXPlayer.cpp | 27 ++++++++++++++++----------- xbmc/cores/omxplayer/OMXPlayer.h | 1 + 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/xbmc/cores/dvdplayer/DVDPlayer.cpp b/xbmc/cores/dvdplayer/DVDPlayer.cpp index 7f6681b7dc18d..4f1f67edef55e 100644 --- a/xbmc/cores/dvdplayer/DVDPlayer.cpp +++ b/xbmc/cores/dvdplayer/DVDPlayer.cpp @@ -810,7 +810,7 @@ void CDVDPlayer::OpenDefaultStreams(bool reset) CloseAudioStream(true); // enable or disable subtitles - SetSubtitleVisible(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn); + SetSubtitleVisibleInternal(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn); // open subtitle stream SelectionStream as = m_SelectionStreams.Get(STREAM_AUDIO, GetAudioStream()); @@ -825,14 +825,14 @@ void CDVDPlayer::OpenDefaultStreams(bool reset) { valid = true; if(it->flags & CDemuxStream::FLAG_FORCED) - m_dvdPlayerVideo.EnableSubtitle(true); + SetSubtitleVisibleInternal(true); } } if(!valid) { CloseSubtitleStream(true); if (m_pInputStream && !(m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD) || m_pInputStream->IsStreamType(DVDSTREAM_TYPE_BLURAY))) - SetSubtitleVisible(false); + SetSubtitleVisibleInternal(false); } // open teletext stream @@ -2255,11 +2255,7 @@ void CDVDPlayer::HandleMessages() else if (pMsg->IsType(CDVDMsg::PLAYER_SET_SUBTITLESTREAM_VISIBLE)) { CDVDMsgBool* pValue = (CDVDMsgBool*)pMsg; - - m_dvdPlayerVideo.EnableSubtitle(pValue->m_value); - - if (m_pInputStream && m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD)) - static_cast(m_pInputStream)->EnableSubtitleStream(pValue->m_value); + SetSubtitleVisibleInternal(pValue->m_value); } else if (pMsg->IsType(CDVDMsg::PLAYER_SET_STATE)) { @@ -2840,6 +2836,15 @@ void CDVDPlayer::SetSubtitleVisible(bool bVisible) m_messenger.Put(new CDVDMsgBool(CDVDMsg::PLAYER_SET_SUBTITLESTREAM_VISIBLE, bVisible)); } +void CDVDPlayer::SetSubtitleVisibleInternal(bool bVisible) +{ + CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn = bVisible; + m_dvdPlayerVideo.EnableSubtitle(bVisible); + + if (m_pInputStream && m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD)) + static_cast(m_pInputStream)->EnableSubtitleStream(bVisible); +} + int CDVDPlayer::GetAudioStreamCount() { return m_SelectionStreams.Count(STREAM_AUDIO); @@ -3178,14 +3183,14 @@ bool CDVDPlayer::AdaptForcedSubtitles() if(OpenSubtitleStream(it->id, it->source)) { valid = true; - SetSubtitleVisible(true); + SetSubtitleVisibleInternal(true); } } } if(!valid) { CloseSubtitleStream(true); - SetSubtitleVisible(false); + SetSubtitleVisibleInternal(false); } } return valid; @@ -3479,7 +3484,7 @@ int CDVDPlayer::OnDVDNavResult(void* pData, int iMessage) int iStream = event->physical_wide; bool visible = !(iStream & 0x80); - SetSubtitleVisible(visible); + SetSubtitleVisibleInternal(visible); if (iStream >= 0) m_dvd.iSelectedSPUStream = (iStream & ~0x80); diff --git a/xbmc/cores/dvdplayer/DVDPlayer.h b/xbmc/cores/dvdplayer/DVDPlayer.h index dfe679f6eb931..79ecec48fef54 100644 --- a/xbmc/cores/dvdplayer/DVDPlayer.h +++ b/xbmc/cores/dvdplayer/DVDPlayer.h @@ -301,6 +301,7 @@ class CDVDPlayer : public IPlayer, public CThread, public IDVDPlayer bool ShowPVRChannelInfo(); int AddSubtitleFile(const std::string& filename, const std::string& subfilename = "", CDemuxStream::EFlags flags = CDemuxStream::FLAG_NONE); + void SetSubtitleVisibleInternal(bool bVisible); /** * one of the DVD_PLAYSPEED defines diff --git a/xbmc/cores/omxplayer/OMXPlayer.cpp b/xbmc/cores/omxplayer/OMXPlayer.cpp index db00d77196930..25b0220b89675 100644 --- a/xbmc/cores/omxplayer/OMXPlayer.cpp +++ b/xbmc/cores/omxplayer/OMXPlayer.cpp @@ -866,7 +866,7 @@ void COMXPlayer::OpenDefaultStreams(bool reset) CloseAudioStream(true); // enable or disable subtitles - SetSubtitleVisible(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn); + SetSubtitleVisibleInternal(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn); // open subtitle stream OMXSelectionStream as = m_SelectionStreams.Get(STREAM_AUDIO, GetAudioStream()); @@ -881,14 +881,14 @@ void COMXPlayer::OpenDefaultStreams(bool reset) { valid = true; if(it->flags & CDemuxStream::FLAG_FORCED) - m_omxPlayerVideo.EnableSubtitle(true); + SetSubtitleVisibleInternal(true); } } if(!valid) { CloseSubtitleStream(true); if (m_pInputStream && !(m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD) || m_pInputStream->IsStreamType(DVDSTREAM_TYPE_BLURAY))) - SetSubtitleVisible(false); + SetSubtitleVisibleInternal(false); } // open teletext stream @@ -2493,11 +2493,7 @@ void COMXPlayer::HandleMessages() else if (pMsg->IsType(CDVDMsg::PLAYER_SET_SUBTITLESTREAM_VISIBLE)) { CDVDMsgBool* pValue = (CDVDMsgBool*)pMsg; - - m_omxPlayerVideo.EnableSubtitle(pValue->m_value); - - if (m_pInputStream && m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD)) - static_cast(m_pInputStream)->EnableSubtitleStream(pValue->m_value); + SetSubtitleVisibleInternal(pValue->m_value); } else if (pMsg->IsType(CDVDMsg::PLAYER_SET_STATE)) { @@ -3097,6 +3093,15 @@ void COMXPlayer::SetSubtitleVisible(bool bVisible) m_messenger.Put(new CDVDMsgBool(CDVDMsg::PLAYER_SET_SUBTITLESTREAM_VISIBLE, bVisible)); } +void COMXPlayer::SetSubtitleVisibleInternal(bool bVisible) +{ + CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn = bVisible; + m_omxPlayerVideo.EnableSubtitle(bVisible); + + if (m_pInputStream && m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD)) + static_cast(m_pInputStream)->EnableSubtitleStream(bVisible); +} + int COMXPlayer::GetAudioStreamCount() { return m_SelectionStreams.Count(STREAM_AUDIO); @@ -3437,14 +3442,14 @@ bool COMXPlayer::AdaptForcedSubtitles() if(OpenSubtitleStream(it->id, it->source)) { valid = true; - SetSubtitleVisible(true); + SetSubtitleVisibleInternal(true); } } } if(!valid) { CloseSubtitleStream(true); - SetSubtitleVisible(false); + SetSubtitleVisibleInternal(false); } } return valid; @@ -3741,7 +3746,7 @@ int COMXPlayer::OnDVDNavResult(void* pData, int iMessage) int iStream = event->physical_wide; bool visible = !(iStream & 0x80); - SetSubtitleVisible(visible); + SetSubtitleVisibleInternal(visible); if (iStream >= 0) m_dvd.iSelectedSPUStream = (iStream & ~0x80); diff --git a/xbmc/cores/omxplayer/OMXPlayer.h b/xbmc/cores/omxplayer/OMXPlayer.h index f34764de7b183..7c76da9ee80dd 100644 --- a/xbmc/cores/omxplayer/OMXPlayer.h +++ b/xbmc/cores/omxplayer/OMXPlayer.h @@ -306,6 +306,7 @@ class COMXPlayer : public IPlayer, public CThread, public IDVDPlayer bool ShowPVRChannelInfo(); int AddSubtitleFile(const std::string& filename, const std::string& subfilename = "", CDemuxStream::EFlags flags = CDemuxStream::FLAG_NONE); + void SetSubtitleVisibleInternal(bool bVisible); /** * one of the DVD_PLAYSPEED defines From 1f125d14fef6952431f38f80c6dd92fd7e6f5ab9 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 9 Feb 2014 20:51:03 +0100 Subject: [PATCH 3/4] dvd/omxplayer Subs couldn't be turned on if no relevant subs existed Original patch by: ace20022 --- xbmc/cores/dvdplayer/DVDPlayer.cpp | 29 ++++++++++++++++++++--------- xbmc/cores/dvdplayer/DVDPlayer.h | 8 -------- xbmc/cores/omxplayer/OMXPlayer.cpp | 29 ++++++++++++++++++++--------- xbmc/cores/omxplayer/OMXPlayer.h | 8 -------- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/xbmc/cores/dvdplayer/DVDPlayer.cpp b/xbmc/cores/dvdplayer/DVDPlayer.cpp index 4f1f67edef55e..1cf1da60dbd56 100644 --- a/xbmc/cores/dvdplayer/DVDPlayer.cpp +++ b/xbmc/cores/dvdplayer/DVDPlayer.cpp @@ -231,17 +231,27 @@ class PredicateSubtitlePriority bool original; bool preferextsubs; bool subson; + PredicateSubtitleFilter filter; public: PredicateSubtitlePriority(std::string& lang) : audiolang(lang), original(StringUtils::EqualsNoCase(CSettings::Get().GetString("locale.subtitlelanguage"), "original")), preferextsubs(CSettings::Get().GetBool("subtitles.preferexternal")), - subson(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn) + subson(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn), + filter(lang) { }; + bool relevant(const SelectionStream& ss) const + { + return !filter(ss); + } + bool operator()(const SelectionStream& lh, const SelectionStream& rh) const { + PREDICATE_RETURN(relevant(lh) + , relevant(rh)); + PREDICATE_RETURN(lh.type_index == CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream , rh.type_index == CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream); @@ -810,31 +820,32 @@ void CDVDPlayer::OpenDefaultStreams(bool reset) CloseAudioStream(true); // enable or disable subtitles - SetSubtitleVisibleInternal(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn); + bool visible = CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn; // open subtitle stream SelectionStream as = m_SelectionStreams.Get(STREAM_AUDIO, GetAudioStream()); - PredicateSubtitleFilter psf(as.language); - streams = m_SelectionStreams.RemoveIf(STREAM_SUBTITLE, psf); PredicateSubtitlePriority psp(as.language); - std::stable_sort(streams.begin(), streams.end(), psp); + streams = m_SelectionStreams.Get(STREAM_SUBTITLE, psp); valid = false; for(SelectionStreams::iterator it = streams.begin(); it != streams.end() && !valid; ++it) { if(OpenSubtitleStream(it->id, it->source)) { valid = true; - if(it->flags & CDemuxStream::FLAG_FORCED) - SetSubtitleVisibleInternal(true); + if(!psp.relevant(*it)) + visible = false; + else if(it->flags & CDemuxStream::FLAG_FORCED) + visible = true; } } if(!valid) { CloseSubtitleStream(true); - if (m_pInputStream && !(m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD) || m_pInputStream->IsStreamType(DVDSTREAM_TYPE_BLURAY))) - SetSubtitleVisibleInternal(false); + visible = false; } + SetSubtitleVisibleInternal(visible); + // open teletext stream streams = m_SelectionStreams.Get(STREAM_TELETEXT); valid = false; diff --git a/xbmc/cores/dvdplayer/DVDPlayer.h b/xbmc/cores/dvdplayer/DVDPlayer.h index 79ecec48fef54..e2a836bbf2833 100644 --- a/xbmc/cores/dvdplayer/DVDPlayer.h +++ b/xbmc/cores/dvdplayer/DVDPlayer.h @@ -154,14 +154,6 @@ class CSelectionStreams return streams; } - template - SelectionStreams RemoveIf(StreamType type, Filter filter) - { - SelectionStreams streams = Get(type); - streams.erase(std::remove_if(streams.begin(), streams.end(), filter), streams.end()); - return streams; - } - void Clear (StreamType type, StreamSource source); int Source (StreamSource source, std::string filename); diff --git a/xbmc/cores/omxplayer/OMXPlayer.cpp b/xbmc/cores/omxplayer/OMXPlayer.cpp index 25b0220b89675..aa1bd76a6fb81 100644 --- a/xbmc/cores/omxplayer/OMXPlayer.cpp +++ b/xbmc/cores/omxplayer/OMXPlayer.cpp @@ -275,17 +275,27 @@ class PredicateSubtitlePriority bool original; bool preferextsubs; bool subson; + PredicateSubtitleFilter filter; public: PredicateSubtitlePriority(std::string& lang) : audiolang(lang), original(StringUtils::EqualsNoCase(CSettings::Get().GetString("locale.subtitlelanguage"), "original")), preferextsubs(CSettings::Get().GetBool("subtitles.preferexternal")), - subson(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn) + subson(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn), + filter(lang) { }; + bool relevant(const OMXSelectionStream& ss) const + { + return !filter(ss); + } + bool operator()(const OMXSelectionStream& lh, const OMXSelectionStream& rh) const { + PREDICATE_RETURN(relevant(lh) + , relevant(rh)); + PREDICATE_RETURN(lh.type_index == CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream , rh.type_index == CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleStream); @@ -866,31 +876,32 @@ void COMXPlayer::OpenDefaultStreams(bool reset) CloseAudioStream(true); // enable or disable subtitles - SetSubtitleVisibleInternal(CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn); + bool visible = CMediaSettings::Get().GetCurrentVideoSettings().m_SubtitleOn; // open subtitle stream OMXSelectionStream as = m_SelectionStreams.Get(STREAM_AUDIO, GetAudioStream()); - PredicateSubtitleFilter psf(as.language); - streams = m_SelectionStreams.RemoveIf(STREAM_SUBTITLE, psf); PredicateSubtitlePriority psp(as.language); - std::stable_sort(streams.begin(), streams.end(), psp); + streams = m_SelectionStreams.Get(STREAM_SUBTITLE, psp); valid = false; for(OMXSelectionStreams::iterator it = streams.begin(); it != streams.end() && !valid; ++it) { if(OpenSubtitleStream(it->id, it->source)) { valid = true; - if(it->flags & CDemuxStream::FLAG_FORCED) - SetSubtitleVisibleInternal(true); + if(!psp.relevant(*it)) + visible = false; + else if(it->flags & CDemuxStream::FLAG_FORCED) + visible = true; } } if(!valid) { CloseSubtitleStream(true); - if (m_pInputStream && !(m_pInputStream->IsStreamType(DVDSTREAM_TYPE_DVD) || m_pInputStream->IsStreamType(DVDSTREAM_TYPE_BLURAY))) - SetSubtitleVisibleInternal(false); + visible = false; } + SetSubtitleVisibleInternal(visible); + // open teletext stream streams = m_SelectionStreams.Get(STREAM_TELETEXT); valid = false; diff --git a/xbmc/cores/omxplayer/OMXPlayer.h b/xbmc/cores/omxplayer/OMXPlayer.h index 7c76da9ee80dd..778aa123f31b0 100644 --- a/xbmc/cores/omxplayer/OMXPlayer.h +++ b/xbmc/cores/omxplayer/OMXPlayer.h @@ -148,14 +148,6 @@ class COMXSelectionStreams return streams; } - template - OMXSelectionStreams RemoveIf(StreamType type, Filter filter) - { - OMXSelectionStreams streams = Get(type); - streams.erase(std::remove_if(streams.begin(), streams.end(), filter), streams.end()); - return streams; - } - void Clear (StreamType type, StreamSource source); int Source (StreamSource source, std::string filename); From 89a028a647130384fef2fb9a351b54c4e3ec6266 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Sun, 9 Feb 2014 12:50:07 +0100 Subject: [PATCH 4/4] dvd/omxplayer: don't override video setting visibility if no subs are found Note: This should be reverted when we have separated GUI display from user specified settings. It temporarily solves the use case of user starting a movie without subs, stop it, add external subtitles, start it again. --- xbmc/cores/dvdplayer/DVDPlayer.cpp | 3 --- xbmc/cores/omxplayer/OMXPlayer.cpp | 3 --- 2 files changed, 6 deletions(-) diff --git a/xbmc/cores/dvdplayer/DVDPlayer.cpp b/xbmc/cores/dvdplayer/DVDPlayer.cpp index 1cf1da60dbd56..5680353113d77 100644 --- a/xbmc/cores/dvdplayer/DVDPlayer.cpp +++ b/xbmc/cores/dvdplayer/DVDPlayer.cpp @@ -839,10 +839,7 @@ void CDVDPlayer::OpenDefaultStreams(bool reset) } } if(!valid) - { CloseSubtitleStream(true); - visible = false; - } SetSubtitleVisibleInternal(visible); diff --git a/xbmc/cores/omxplayer/OMXPlayer.cpp b/xbmc/cores/omxplayer/OMXPlayer.cpp index aa1bd76a6fb81..c01e3b10deff1 100644 --- a/xbmc/cores/omxplayer/OMXPlayer.cpp +++ b/xbmc/cores/omxplayer/OMXPlayer.cpp @@ -895,10 +895,7 @@ void COMXPlayer::OpenDefaultStreams(bool reset) } } if(!valid) - { CloseSubtitleStream(true); - visible = false; - } SetSubtitleVisibleInternal(visible);