Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

DVDPlayer: try to find an audiostream that matches the GUI/DVD language #700

Merged
merged 4 commits into from

7 participants

@Montellese
Owner

This commit changes the way CDVDPlayer chooses the default audio stream. Up until now we choose the audio stream with the default flag and if there's no default flag we try the one with the most channels/best codec and if that fails we choose the first one available.

With these changes the behaviour will be like this:
1. Sort all available audio streams by number of channels and codec with the "best" being first and the "worst" being last.
2. Go through the sorted streams and check if there's one stream matching the GUI/DVD language of the current XBMC installation. Try all the ones with a matching language starting with the "best" until we find one that works.
3. If there's no matching/working audio stream based on the language we look for the default stream. If there is one use it.
4. If there is no default stream start with the "best" stream and work your way down towards the worst until we find a stream that works.

So all in all we have two improvements (at least for me those are improvements):

  • If possible we use the audio stream with the appropriate language
  • If there's no default stream we use the "best" working audio stream. The way it is now we only try the best and if that doesn't work out we don't use the second best, instead we use the first stream. So if there are three audio streams, one with DTS-HD, one with DD5.1 and one with MP3 we currently choose the best one (DTS-HD) but if that fails we try the first one, which might be MP3. With these changes we first try DTS-HD, then DD5.1 and then at last MP3.
@jmarshallnz
Owner

Should we use the default stream in preference to the "best" stream?

With this we're overriding default stream for language. Should we also override the default stream if there's a "better" one? Dunno :)

Otherwise, the only thing I don't really like is the sort when std::sort exists - eg you could sort according to whether it matches the assigned language, then by default, then by best (swap the last two around according to the above) - it does mean passing a param to the sort predicate though. No doubt you've considered this already and have good reasons why a sort won't work though :)

@Montellese
Owner

The order of what should be checked first is certainly debatable. Generally something is marked as the default stream for a reason. I just put the language before the default stream because that's how I'd want it to behave. But if everyone else thinks differently I can change it. I checked a few of my multi-language MKVs and they all have a default audio stream set.

Yeah I don't really like doing the sorting myself either. The reasons why I did it that way is that I did not want to change the original order of the streams as read from the container. Furthermore I figured that in most cases there are one or two streams to sort so using std::sort doesn't really seem like it will provide a huge advantage. But if we don't really need/care about the original order of the streams I'm happy to remove the custom sorting code, implement the needed method to pass to std::sort und use that one. I'm guessing @elupus will be the man to tell me whether we want to keep the original order or not.

@jmarshallnz
Owner

Agreed that the number of items being sorted isn't a big deal.

I'm not sure what the default stream means either.

@elupus
Collaborator
@Montellese
Owner

So I'll add an option to prioritize choosing the audio stream based on the language (if possible) over a default audio stream?

@Montellese
Owner

OK I pushed a commit that adds a GUI setting (off by default to keep current behaviour) "Prioritise audio streams matching the user interface language" to Video -> Playback but I have to admit I'm not at all happy with that approach/solution. I think it's a good idea to let the user choose what should be looked at first when choosing an audio stream but currently there are 5 different checks:
1. If this video has been played before use the streams that were used the last time (this should always be 1 IMO)
2. Use the default stream
3. Try to find a stream matching the GUI language
4. Use the stream with the most channels / best codec
5. USe the first stream

While 1 and 5 are perfectly sane where they are, 2, 3 and 4 are kind of interchangable which results in 6 possible cases. Probably not all of them make sense (but you never know, there's always someone which prefers some very odd setup). Does anybody have a better idea how to make this more flexible for the user to choose or do you think it's not worth it?

@elupus
Collaborator

Can't we combine the settings? Make it possible to select language "Original" instead of an explicitly set language? Think that would make more sense.

@Montellese
Owner

I'm not sure I understand. With "original" language you mean the default stream? What would be the difference to the implementation I posted yesterday (apart from it being formulated the other way around) which selects an explicitly set language (GUI/DVD language) over the original one?

@elupus
Collaborator

I technically want to avoid another setting. But I thought we already had a setting for dvd language, but seem we piggy back on GUI language so another setting is probably appropriate.

This causes a bit of an issue thou. I for example (and many other swedes) run xbmc in English, but if we ever want subtitles we'd most likely want them in Swedish. Ie not gui setting.

I think we should have two scroll boxes for "Prefered Audio Language" and "Prefered Subtitle Language", these should contain languages + a gui language option and a original movie language option.

@Montellese
Owner

Actually I didn't use the GUI language. What XBMC does is it looks into the langinfo.xml that is part of the directory containing the strings.xml of the language you chose in XBMC. There you'll find

<dvd>
    <menu>en</menu>
    <audio>en</audio>
    <subtitle>en</subtitle>
</dvd>

So I figured it would make sense to use that for selecting audio streams in CDVDPlayer as well. I also run XBMC in english (although I'm german) so what I did was I modified my langinfo.xml so that DVD menus etc are displayed in german and not english. But I agree that this solution is far from user-friendly as it involves modifying an XML file. So maybe the two new settings you propose would then override the settings read from langinfo.xml and would also apply to DVDs. Where should those be put? In Appearence -> International where a user already specifies the GUI language or in Video -> Playback?

@elupus
Collaborator
@jmarshallnz
Owner
@Montellese
Owner

OK I just updated the last commit (removed the previous GUI setting and added a new one) which adds a new GUI setting "Preferred audio language" in Appearence -> International. IT allows to choose between "original" (the default stream), "default" (GUI language) and all other languages for which XBMC offers translations. I didn't remove the stuff from langinfo.xml yet (and I even use CLangInfo::GetDVDAudioLanguage() for now). There are two problems with the current implementation:

  1. I think I/we need to come up with better strings than "original" and "default" but I couldn't think of any right now.
  2. There is a very bothersome delay between the moment I change the value of the setting and the time it is actually changed inside XBMC. This goes as far as that it's possible to change the value and then quickly press ESC to leave the settings dialog which causes the just changed value to not being stored in CGUISettings. You have to wait a short period to be sure that it has been stored. While this makes sense for the GUI language setting (we don't want to unload all translations and load the new ones everytime the setting is changed because in most cases the user has to go through several languages to arrive at the one he would like to use), this is very bothersome for the preferred audio language setting. During my testing I had to go back to the settings dialog and change the value again because I pressed ESC too soon after changing the preferred audio language setting.

If you are ok with the implementation for the preferred audio language (and the above mentioned problems are solved) I will add another setting for the preferred subtitle language (which will probably need some discussing as well because there we also have external subtitles to consider).

xbmc/cores/dvdplayer/DVDPlayer.cpp
((32 lines not shown))
- if(s.source == STREAM_SOURCE_DEMUX) // Only demux streams
+ /*
+ * If there is more than one audio stream and a valid one is not chosen yet, select the one
+ * with a matching language. If there are multiple matches for the language, select the one
+ * with the maximum number of channels or the best codec if the same number of channels.
+ */
+ CStdString audio_language_twochar = g_langInfo.GetDVDAudioLanguage();
+ // If the audio language is not set to "default" we need to get the DVDAudioLanguage of the
+ // language that was actually set
+ if (!g_guiSettings.GetString("locale.audiolanguage").Equals("default"))
+ {
+ CStdString strLangInfoPath;
+ strLangInfoPath.Format("special://xbmc/language/%s/langinfo.xml", g_guiSettings.GetString("locale.audiolanguage").c_str());
+ CLangInfo langInfo;
+ if (langInfo.Load(strLangInfoPath))
+ audio_language_twochar = langInfo.GetDVDAudioLanguage();
@elupus Collaborator
elupus added a note

This seem like something that should be done on xbmc startup, not on every file start.

@Montellese Owner

It could be stored in g_langInfo instead of the value read from langinfo.xml (if we want to drop those) so that it can be retrieved using g_langInfo.GetDVDAudioLanguage() (or rename it to GetAudioLanguage).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/cores/dvdplayer/DVDPlayer.cpp
@@ -292,6 +293,36 @@ void CSelectionStreams::Update(CDVDInputStream* input, CDVDDemux* demuxer)
}
}
+void CSelectionStreams::GetAudioSortByChannelsAndCodec(std::vector<int> &order)
+{
+ order.clear();
+
+ for(int i = 0; i < Count(STREAM_AUDIO); i++)
+ {
+ SelectionStream& s = Get(STREAM_AUDIO, i);
+ if(s.source == STREAM_SOURCE_DEMUX) // Only demux streams
@elupus Collaborator
elupus added a note

What happens here with dvd's? stream source is not demux then if i remember correctly

@Montellese Owner

I have to admit that I don't know. I took this from the current logic which chooses the audio stream with the most channels / best codec (see https://github.com/xbmc/xbmc/pull/700/files#L1L640).

@Montellese Owner

I just checked the code and I'm not sure if this code does even affect DVDs because there is some code in DVDInputStreamNavigator.cpp which tries to set the audio stream using DllDvdNav::dvdnav_audio_language_select() (where we probably should replace the usage of CLangInfo::GetDVDAudioLanguage() by the new CLangInfo::GetAudioLanguage()).

@elupus Collaborator
elupus added a note

Right. That is true. The navigator will take care of these stuff then, so we should not be trying to select anything here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Montellese
Owner

Another rebase after an input from elupus (thanks for that). I added methods GetAudioLanguage() and SetAudioLanguage() to CLangInfo which get/set the preferred audio language as set by the user through the GUI setting. This way the language's locale doesn't have to be determined everytime something is played through CDVDPlayer.

I checked most langinfo.xml files of the different languages XBMC supports and in all of them the dvdmenu, audio and subtitle languages match the locale of the language itself. So I think by adding the preferred audio/subtitle GUI settings those would be safe to remove/replace.

@elupus elupus was assigned
@Montellese
Owner

So I'd like to bump this one. Reading through the discussion so far these are the things that still seem to need further discussion:

  1. We need to come up with better names for the current "Original" (default stream) and "Default" (GUI language) strings in the audio stream language setting. Suggestions are welcome ;-)
  2. We need to discuss whether we want to drop the stuff from langinfo.xml. I've noticed that for most of the languages the languages definded in the tags match the language itself but for a select few (e.g. Afrikaans) they don't match (for Afrikaans the language's locale is "za" but the languages defined in are "en") so I'm not sure what to do about that.
  3. I still don't know if the last comment from @elupus (see https://github.com/xbmc/xbmc/pull/700/files#L3R303) is correct and needs some code change or not.
  4. Should the new CLangInfo::GetAudioLanguage() which is based on the GUI setting replace the currently used CLangInfo::GetDVDAudioLanguage() (and probably CLangInfo::GetDVDMenuLanguage()) or not?
  5. This is not really related to this PR itself, I just noticed it when using the audio stream language setting: There is a very bothersome delay between the moment I change the value of the setting and the time it is actually changed inside XBMC. This goes as far as that it's possible to change the value and then quickly press ESC to leave the settings dialog which causes the just changed value to not being stored in CGUISettings. You have to wait a short period to be sure that it has been stored. While this makes sense for the GUI language setting (we don't want to unload all translations and load the new ones everytime the setting is changed because in most cases the user has to go through several languages to arrive at the one he would like to use), this is very bothersome for the preferred audio language setting. During my testing I had to go back to the settings dialog and change the value again because I pressed ESC too soon after changing the preferred audio language setting.

I'd also volunteer to do similar work for subtitle selection once this PR has been done and merged.

@jmarshallnz
Owner

For 5, I agree - there's no need for this one not to be instant. The bit making it not instant is in FillInLanguages()

setting->SetDelayed();

Simple fix I guess is moving that bit out into the locale.language handler.

@jmarshallnz
Owner

For 1, you could always just use "Same as UI" or some such for the second. The first isn't too bad I think?

@jmarshallnz
Owner

For 4, I'd replace, yes, but obviously defer to @elupus in this respect.

For 2, what does langinfo.xml offer over and above the setting you have?

@elupus
Collaborator

Yea i think GetDVD(Audio/Menu)Language is rather pointless after this. GUI language seems fine by me in the situation that you don't want the movies native language.

I didn't check,but is FORCED still handled? It need to select that stream if subtitles are turned of.

@Montellese
Owner

For 1: Yeah "Original" isn't that bad as long as the other isn't "Default" because it could be understood as "Default stream".
For 2: Well like I mentioned langinfo.xml sometimes provides different locales than the language's locale (as for Afrikaans as it's probably hard to find a DVD with a menu in Afrikaans or anything like that). So it will probably change the behaviour for certain languages if we pull from langinfo.cml
For 3: Thanks for the comment elupus, so that should be fine.
For 5: Thanks for the hint, I'll see how I can refactor it.

@elupus: This doesn't handle subtitles yet and AFAIK there's no forced flag for audio streams right?

@elupus
Collaborator
@Montellese
Owner

OK I have looked into doing the same thing for subtitles and here is how it could work:

  1. If subtitles are disabled and there's a FORCED subtitle, use that one independant of the language (can there be multiple subtitles with the FORCED flag set?)
  2. If there are saved settings for the video, stream and subtitles use the same settings again
  3. If there are external subtitles, try to find one that matches the preferred language and use it if available
  4. If there's no external subtitle matching the language, use the first external subtitle
  5. If there is a preferred subtitle language specified, try to find an internal subtitle that matches the preferred language and use it if available
  6. If there is no preferred subtitle language specified or non of the subtitles available match the preferred language, see if there is a subtitle with the DEFAULT flag and use that one if available
  7. If all that fails, use the first subtitle that can be opened/read

The selection procedure gets a bit complicated (but it already is complicated now because of FORCED, saved settings, external and DEFAULT) but I think it should work.

Once that has been added and works, I'll add another commit to drop the from langinfo.xml and the respective CLangInfo::GetDVDFooLanguage() methods.

@Montellese
Owner

OK I have reworked/refactored the whole implementation. Thanks to that I noticed a bug on win32 where for certain locales like German the three char code is "deu" instead of "ger" which is used by linux and by MKV. So I had to add an extra method to CLangCodeExpander.

I haven't removed the GetDVDFooLanguage() methods from CLangInfo yet. Doing that will require another helper method to get the two char language code (like "en") from the three char language code (like "eng") because addons etc use the two char language code. But that should be pretty easy to come up with and I'm happy to do so. Should I add it to this PR or should I create a new one once this has been merged?

@Montellese
Owner

Forgot to also change the strings I didn't like. I renamed them to "Original stream's language" and "User Interface language". Let me know if you are ok with those or not.

@Montellese
Owner

Any chance I can get the latest changes & rebase reviewed (once again) so it could be merged in May? Thanks for your patience.

@Uxorious

There really should be a UI option to pick the preferred behavior.
With so many users/nationalities/preferences, there is no way to pick a system behavior that will satisfy everybody.

The one thing I agree with in the discussion above is that if the user changed the audio track for the file, it should pick that track again on subsequent plays.

I would love to see a new option in the UI ... lets call it "Intelligent track picking".

For choices in it, I could see:
1) OFF (XBMC <= v10 behavior I think).
2) "Best track" (XBMC v11 behavior ... except why is AAC a worse codec than AC3 currently).
3) Original language (Match audio language to video language ... some of us hate dubbing).
4) User language (Match audio to GUI language ... some of us prefer dubbing).

There could also be an argument for having a "Preferred language order" (some of us know more than one language and would be fine with those).

At any rate, I'm looking forward to having any way to improve the current (v11) situation. Thanks!

@Montellese
Owner

Well like I mentioned earlier the current implementation offers a setting for both audio and subtitle language. The available options are

1) Original language (whatever is the default/first audio stream in the video container).
2) User Interface language
3) Any specific language

In all of these cases XBMC uses it's "best track" detection to choose the best track in case there are multiple tracks with the same language.

A "preferred language order" would be a lot more work to implement and would make it more complicated.

Concerning AAC vs AC3 I don't really care because none of my videos have AAC. Changing that logic is also not part of this PR so I won't touch it.

xbmc/cores/dvdplayer/DVDPlayer.cpp
((17 lines not shown))
+ {
+ SelectionStream& compStream = Get(STREAM_AUDIO, order[j]);
+ if(s.channels > compStream.channels
+ ||(s.channels == compStream.channels && codec_priority > StreamUtils::GetCodecPriority(compStream.codec)))
+ {
+ order.insert(order.begin() + j, i);
+ added = true;
+ break;
+ }
+ }
+
+ if(!added)
+ order.push_back(i);
+ }
+ }
+}
@elupus Collaborator
elupus added a note

This looks like this could be handled with std::sort() with a sorting operator.

@Montellese Owner

@jmarshallnz has already mentioned this earlier and this was my response:

I don't really like doing the sorting myself either. The reasons why I did it that way is that I did not want to change the original order of the streams as read from the container. Furthermore I figured that in most cases there are one or two streams to sort so using std::sort doesn't really seem like it will provide a huge advantage. But if we don't really need/care about the original order of the streams I'm happy to remove the custom sorting code, implement the needed method to pass to std::sort und use that one. I'm guessing @elupus will be the man to tell me whether we want to keep the original order or not.

So if we don't need to keep the original order I can switch to using std::sort. Otherwise I'd either have to use the custom sorting implementation or copy the whole CSelectionStreams object into a new one in which I can change the order (because it's just a copy).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elupus
Collaborator

No major complaints from my side

@da-anda
Collaborator

as for the select options

"Prefered audio language:"

  • Movie default
  • same as user interface
  • custom

If "custom" is selected, display a second dropDown with the language selector.

@elupus
Collaborator
@Montellese
Owner

@da-anda I don't see why a second dropDown is necessary if it's doable (and already done) with only one (Original language, UI language, Afrikaans, ...., German, ..., English, ...).

@elupus IIRC stable_sort only keeps the relative order of items which are "equal" but if all the items differ, they are still completely re-ordered.

@jmarshallnz
Owner

Given you're returning the sort order not the items sorted I agree that doing it the way you have is best. I'm not sure whether it makes sense to reorder the streams themselves. @elupus?

@elupus
Collaborator

Montellese, if you don't want to sort items that are different then i don't understand what it is doing. For std::sort_order you define what different is. So if you want order preserved it will be preserved.

@elupus
Collaborator

I don't see this as blocking btw. It's not especially complex code, so it's fine as it is with the sorting.

@jmarshallnz
Owner

The only question is whether he actually reorders the streams, or instead returns the order (i.e. a vector of indicies that are in the order he wants).

ATM he's doing the latter.

Question is whether the streams should instead be reordered (and thus std::stable_sort() could be used).

@elupus
Collaborator

I just had to give it a go on the current code. I think adding that CSelectionStream::Get(type) returning a vector of all the streams make sense anyway.

diff --git a/xbmc/cores/dvdplayer/DVDPlayer.cpp b/xbmc/cores/dvdplayer/DVDPlayer.cpp
index 3658921..4c1a9ae 100644
--- a/xbmc/cores/dvdplayer/DVDPlayer.cpp
+++ b/xbmc/cores/dvdplayer/DVDPlayer.cpp
@@ -116,6 +116,25 @@ SelectionStream& CSelectionStreams::Get(StreamType type, int index)
   return m_invalid;
 }

+std::vector<SelectionStream> CSelectionStreams::Get(StreamType type)
+{
+  std::vector<SelectionStream> streams;
+  int count = Count(type);
+  for(int index = 0; index < count; ++index){
+    streams.push_back(Get(type, index));
+  }
+  return streams;
+}
+
+static bool PredicateAudioPriority(const SelectionStream& lh, const SelectionStream& rh)
+{
+  if(lh.channels > rh.channels)
+    return true;
+  if(StreamUtils::GetCodecPriority(lh.codec) > StreamUtils::GetCodecPriority(rh.codec))
+    return true;
+  return false;
+}
+
 bool CSelectionStreams::Get(StreamType type, CDemuxStream::EFlags flag, SelectionStream& out)
 {
   CSingleLock lock(m_section);
@@ -630,34 +649,16 @@ void CDVDPlayer::OpenDefaultStreams()
        * If there is more than one audio stream and a valid one is not chosen yet, select the one
        * with the maximum number of channels or the best codec if the same number of channels.
        */
-      int max_channels = -1;
-      int max_codec_priority = -1;
-      CStdString max_codec;
-      int max_stream_id = -1;
-      for(int i = 0; i < count; i++)
-      {
-        SelectionStream& s = m_SelectionStreams.Get(STREAM_AUDIO, i);
-        if(s.source == STREAM_SOURCE_DEMUX) // Only demux streams
-        {
-          int codec_priority = StreamUtils::GetCodecPriority(s.codec);
-          if(s.channels > max_channels
-          ||(s.channels == max_channels && codec_priority > max_codec_priority))
-          {
-            max_channels = s.channels;
-            max_codec_priority = codec_priority;
-            max_codec = s.codec;
-            max_stream_id = i;
-          }
-        }
-      }
-      if(max_stream_id >= 0)
+      std::vector<SelectionStream> streams = m_SelectionStreams.Get(STREAM_AUDIO);
+      std::stable_sort(streams.begin(), streams.end(), PredicateAudioPriority);
+      for(int i = 0; i<streams.size() && !valid; i++)
       {
-        SelectionStream& s = m_SelectionStreams.Get(STREAM_AUDIO, max_stream_id);
+        SelectionStream& s = streams[i];
         if(OpenAudioStream(s.id, s.source))
         {
           valid = true;
           CLog::Log(LOGDEBUG, "%s - using automatically selected audio stream (%d) based on codec '%s' and maximum number of channels '%d'",
-                    __FUNCTION__, s.id, max_codec.c_str(), max_channels);
+                    __FUNCTION__, s.id, s.codec.c_str(), s.channels);
         }
         else
           CLog::Log(LOGWARNING, "%s - failed to open automatically selected audio stream (%d)", __FUNCTION__, s.id);
diff --git a/xbmc/cores/dvdplayer/DVDPlayer.h b/xbmc/cores/dvdplayer/DVDPlayer.h
index 963de24..f03ef34 100644
--- a/xbmc/cores/dvdplayer/DVDPlayer.h
+++ b/xbmc/cores/dvdplayer/DVDPlayer.h
@@ -138,6 +138,9 @@ public:
   SelectionStream& Get     (StreamType type, int index);
   bool             Get     (StreamType type, CDemuxStream::EFlags flag, SelectionStream& out);

+  std::vector<SelectionStream> Get(StreamType type);
+  std::vector<SelectionStream> GetSorted(StreamType type);
+
   void             Clear   (StreamType type, StreamSource source);
   int              Source  (StreamSource source, std::string filename);
@elupus
Collaborator

AAC is probably worse since it can't be passed through.

@elupus
Collaborator

After some pondering. This fully take care of the default audio stream selection too: so i updated the patch: http://paste.ubuntu.com/968682/

@elupus
Collaborator

It could take care of putting previously selected on top too i think if the SelectionStream struct contained the original index of the audio stream. Ps. this is fully untested currently.

@elupus
Collaborator

Soo.. apperently i was a bit bored: http://paste.ubuntu.com/968721/

Extended it to support video and subtitles too. If we just could get that damn previously selected piece in there too it would end up quite clean.

It should be easy to extend those predicate functions what you need in this pull request to select the right track.

@Montellese
Owner

@elupus Apperently you were ;-) I'll take a look at it and see how well it integrates with my changes. Thanks for the work.

@elupus
Collaborator

Still bored it seems, so more complete version that also handles selected stream: http://paste.ubuntu.com/968886/
just in the process of some testing.

@Uxorious

elupus: AAC is probably worse since it can't be passed through.

Actually it can ... but no amplifiers seem to support it :-)

At any rate, if that is the reasoning, then the selection should really also take a look at the HW configuration.
In my case, it's hooked up with HDMI (supporting 7.1 PCM), so passthrough is a non-issue.

@Montellese
Owner

@elupus I took a look at your code and I'm not sure if your PredicateAudioPriority and PredicateSubtitlePriority are correct. As an example if we have two audio streams, stream A has more channels than stream B but stream B uses the better codec. If we step through PredicateAudioPriority comparing A against B we will return true on the "if(lh.channels > rh.channels)". If we then compare B against A the "if(lh.channels > rh.channels)" check obviously fails but the "if(StreamUtils::GetCodecPriority(lh.codec) > StreamUtils::GetCodecPriority(rh.codec))" check succeeds and returns true as well which (at least on win32) triggers an assert.

Otherwise I really like the approach of the implementation because it cleans up OpenDefaultStreams a lot.

@Montellese
Owner

Forgot to mention that I integrated the language priorization into your approach: Montellese@master...dvdplayer_default_audio_language_new
But it triggers the same assert if there's a stream A matching in language and a stream B not matching in language but with a better codec or more channels.

@elupus
Collaborator
@da-anda
Collaborator

do we need a checkbox "prefer language over quality" like other software has it?

@Montellese
Owner

No we don't. If you select "Original stream" it will automatically look for the best quality stream.

@Uxorious

I guess I will find out if it will work for my files or not ... the "look for the best quality stream" scares me.

Will there be an option to just do what v10 did ... always play the first/default track?

@Montellese
Owner

Please read through my previous comments and explanations. There is one which details exactly how this works. And in the end there will always be someone who doesn't like a feature and we/I don't want to add X extra settings so that really every user gets his way but 99% of the users are overwhelmed with all the different settings.

@Uxorious

I did read through it, but its not obvious what the behavior will be.
For example, lets say I have an MP4 file:
Video = German
Audio 1 = German, AAC, 5.1
Audio 2 = English, AC3, 5.1
UI is set to English.

In XBMC v10, it plays Track 1.
In XBMC v11, it plays Track 2.

With your 3 options what will it play?
1) Original language (whatever is the default/first audio stream in the video container).
Will it play German?
2) User Interface language
Will it play English?
3) Any specific language
No idea what this will do - but I probably wouldn't use it anyways.

@Montellese
Owner

1) The option is called "Original stream" and ignores the streams language completely. So it will look if there is a stream marked with the DEFAULT flag. If Audio 1 has the DEFAULT flag, it will use Audio 1. If Audio 2 has the default flag it will use Audio 2. If none have the default flag it will use the "best" audio stream (based on channels and codec).
2) Yes it will play the english stream Audio 2.
3) If you set the language to German, it will play Audio 1. If you set a language which doesn't match any stream it will behave like 1)

@Uxorious

That's exactly what I feared ... it will not work for me :-(

1) The option is called "Original stream" and ignores the streams language completely.

So there are no options that actually looks at the language WITHOUT forcing anything?

So it will look if there is a stream marked with the DEFAULT flag.
The problem is that MP4 does not have any way to mark a "default" stream ... the first track is the default.

If none have the default flag it will use the "best" audio stream (based on channels and codec).
How about making a change so that MP4 treats the first track as the DEFAULT one?

Or how about making the track selection consider the language as welI ... so if the first track is marked as German, then only consider other German tracks for replacement?

@Montellese
Owner

Sorry but I still don't understand your problem (apart from that you think AAC is better than AC3). If you leave the new options at their default value "Original stream" everything will work exactly the same as it does in Eden.

What would be the use of looking at the language without forcing anything? Why would I want to look at the language in the first place if I don't use it in the decision process?

That's why I use the MKV container. It has the best support for all those flags like FORCED, DEFAULT etc. I don't know how MP4 is handled. But just adding a special handling for MP4 just for you does not sound like a valid option to me (and even if it is I won't be the one to code it because I don't have any MP4s). Even though MKV has the possibility to set a default stream, you don't have to. It's more like a "You should really use this stream" (maybe because it has surround-sound while the other streams are stereo). Why do you have "secondary" streams encoded with AC3 (whereas the "primary" stream uses AAC) if you prefer AAC anyway? Doesn't really make sense to me.

At the end of the day, if you don't like how the newly added functionality works, just don't use it and leave it at its default value. If you don't like how the current code (in Eden and in master) works you already have this problem now and it won't get worse. You also should consider that I was/am the person who took the time to write this code so obviously I coded it so that it works the way I want it to. If one user (out of many many) has requirements that completely conflict with my needs I can't do much about it.

@Montellese
Owner

Updated my alternative branch to the laste code from @elupus at Montellese@master...dvdplayer_default_audio_language_new but I couldn't test it yet. Will report back once I was able to test it.

@da-anda
Collaborator

Uxorious - all you have to do is to select "German" as prefered audio language and you will always get the "best" german audio track by default.

@Montellese
would it be hard to add a checkbox to enable/disable the "quality" detection, or is this something for a separate PR?
While I think the quality detection is nice to have for movies, it's not at all f.e. for liveTV (slower zapping, and many times it's just 2c-AC3 so no real improvement with the downside of not beeing able to change the volume). So there might be a certain demand for beeing able to disable it in some setups.

@Montellese
Owner

Being able to disable the quality detection is not part of this PR (and as I like it I won't change it) and PVR is still not part of master.

xbmc/LangInfo.cpp
((33 lines not shown))
+
+// three char language code (not win32 specific)
+const CStdString& CLangInfo::GetSubtitleLanguage() const
+{
+ if (!m_subtitleLanguage.empty())
+ return m_subtitleLanguage;
+
+ return m_languageCodeGeneral;
+}
+
+void CLangInfo::SetSubtitleLanguage(const CStdString &language)
+{
+ if (language.size() == 2)
+ {
+ if (!g_LangCodeExpander.ConvertTwoToThreeCharCode(m_subtitleLanguage, language))
+ m_audioLanguage.clear();
@da-anda Collaborator
da-anda added a note

shouldn't it be "m_subtitleLanguage.clear();"?
Also, as this is more or less a code duplication, can't a generic method be used to do the conversion?
Like "m_subtitleLanguage = CLangInfo::ConvertToThreeCharLanguageCode(language);"

@Montellese Owner

A c&p error. Thanks for spotting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/cores/dvdplayer/DVDPlayer.cpp
((30 lines not shown))
{
- SelectionStream& s = m_SelectionStreams.Get(STREAM_AUDIO, i);
- if(s.source == STREAM_SOURCE_DEMUX) // Only demux streams
+ /*
+ * If there is more than one audio stream and a valid one is not chosen yet, select the one
+ * with a matching language. If there are multiple matches for the language, select the one
+ * with the maximum number of channels or the best codec if the same number of channels.
+ */
+ CStdString audio_language = g_langInfo.GetAudioLanguage();
+ if(audio_language.size() == 2)
@da-anda Collaborator
da-anda added a note

isn't this check unnecessary, given the fact that g_langInfo.GetAudioLanguage will always return a 3char code if not empty?

@Montellese Owner

It has been removed in the new code in my alternative branch (see the PR's last comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
xbmc/cores/dvdplayer/DVDPlayer.cpp
@@ -703,6 +749,32 @@ void CDVDPlayer::OpenDefaultStreams()
CLog::Log(LOGWARNING, "%s - failed to restore selected subtitle stream (%d)", __FUNCTION__, g_settings.m_currentVideoSettings.m_SubtitleStream);
}
+ bool match_subtitle_language = false;
+ CStdString subtitle_language;
+ if(!g_guiSettings.GetString("locale.subtitlelanguage").Equals("original"))
+ {
+ subtitle_language = g_langInfo.GetSubtitleLanguage();
+ if(subtitle_language.size() == 2)
@da-anda Collaborator
da-anda added a note

as above - this check should be unnecessary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Uxorious

Montellese: Sorry but I still don't understand your problem (apart from that you think AAC is better than AC3).
If you leave the new options at their default value "Original stream" everything will work exactly the same as
it does in Eden.

Yeah but if you read my comments, you would realize that Eden broke the behavior from Dharma.

And yes, AAC is arguably a better quality codec than AC3.

Montellese: What would be the use of looking at the language without forcing anything?
Why would I want to look at the language in the first place if I don't use it in the decision process?

The problem is that right now Eden ignores the language in its decision process!

That's why I use the MKV container. It has the best support for all those flags like FORCED, DEFAULT etc.

MKV is a lovely format, but I have devices that don't support it.

I don't know how MP4 is handled. But just adding a special handling for MP4 just for you does not sound like
a valid option to me (and even if it is I won't be the one to code it because I don't have any MP4s).

I am not proposing special handling for MP4s.
I am proposing an improvement to your pull request.

Why do you have "secondary" streams encoded with AC3 (whereas the "primary" stream uses AAC)
if you prefer AAC anyway? Doesn't really make sense to me.

Who says I made the files?

If you don't like how the current code (in Eden and in master) works you already have this problem now and it
won't get worse. You also should consider that I was/am the person who took the time to write this code so
obviously I coded it so that it works the way I want it to. If one user (out of many many) has requirements that
completely conflict with my needs I can't do much about it.

The sole reason I am here is because I really appreciate that you are working in improving the situation.
Unfortunately you seem to believe that what I am talking about completely conflicts with what you are doing, when likely it would have zero impact on you and your needs.

The way Eden works right now (your Original stream mode), language is completely ignored, and XBMC attempts to pick the "best quality" track - ignoring language.
The reason you don't mind that mode is that all your files have the DEFAULT track set ... something MP4 doesn't support.

What I am proposing is to make "Original track" do the following:
1) If a different track was previously selected, then use that.
2) If a DEFAULT is set, then use that.
3) Search for the "best quality" track amongst all the tracks in the file *but restrict it to either the language of the video, or of the first audio track".

da-anda: Uxorious - all you have to do is to select "German" as prefered audio language and you will always get
the "best" german audio track by default.

I guess my example was too specific.
The usecase was meant to illustrate people who like to watch a movie in its "original language".
If I watch a French movie, I want the French audio. Watch a German one, and I want the German audio.
If I am lucky, I understand the language ... if not, I have the subtitles.

Sure I can go change the UI to force the language every time I want to watch something else, but thats hardly convenient.

@da-anda
Collaborator

so you like to watch the movie in it's native language by default, while the audio track with the native language might not be the first/default one? So language detection should consider the video streams language flag? Or would it be ok to just use the first audio track if no "default" is set? The later could be achived by setting the language mode to "original" and making the "quality check" optional, which won't be part of this PR as Montellese said. So that would be a separate feature/pull request.

@Uxorious

so you like to watch the movie in it's native language by default, while the audio track with the native language
might not be the first/default one?

Almost.
Yes I like watching a movie in the original language.
And that language IS the first track!

The problem is that since MP4 does not support a DEFAULT language, everything gets ignored.
Say I have a file:
Video:German.
Audio 1: German, AAC 5.1
Audio 2: Chinese (dub), AC3 5.1

In this case, it will currently play the Chinese track since AC3 is "better" than AAC.
Any human (who doesn't happen to speak Chinese) looking at the tracklist would probably not consider the 2nd track inherently better than the first one.

So language detection should consider the video streams language flag?

I would say any attempt to make "clever" selection should not ignore the language.

Or would it be ok to just use the first audio track if no "default" is set?
The later could be achived by setting the language mode to "original" and making the "quality check"
optional, which won't be part of this PR as Montellese said. So that would be a separate feature/pull request.

Yes that could work too - but the "clever" selection being done in Montellese's change would just not be terribly clever.
Also, the name "Original stream" would be a bit misleading ... "Highest quality stream" would be more apt.

@Montellese
Owner

OK I rebased my work on top of the work done by @elupus (nice work btw) to achieve with this PR what I initially intended. Any additional logic should be implemented in a different PR so please create a (or several) ticket for whatever additional features you guys would like to see and provide detailed descriptions.

After a final review I'd like to pull this in during this (May) merge window. Thanks.

@da-anda
Collaborator

Montellese, would it be an option to detect and use the language of the first audio-stream found in the file when no specific language was specified in the GUI settings? So that when the setting is "original" that it checks for the stream in the best quality but limited to the language of the first stream (if defined)? It actually doesn't make sense to pick just any stream quality not taking the language into account. Or should this also go into a separate PR in your eyes?

@elupus
Collaborator
@Montellese Montellese was assigned
@Montellese Montellese merged commit c5ffa0e into xbmc:master
@Uxorious

Montellese, would it be an option to detect and use the language of the first audio-stream found in the file when
no specific language was specified in the GUI settings? So that when the setting is "original" that it checks for
the stream in the best quality but limited to the language of the first stream (if defined)?

Unless one of you guys volunteer to make the change, I guess I will sync the code and work on this change request...
I'm not terribly familiar with the codebase, but hopefully the change won't be too involved.

@Montellese
Owner

Remembering the language of the first stream will be a bit difficult because the sorting of all the available streams is done in a stateless way. An easier approach would be (as already described by someone before) a toggle setting to disable the "best quality" selection and just go for the DEFAULT or the first stream.

@Uxorious

Remembering the language of the first stream will be a bit difficult because the sorting of all the available
streams is done in a stateless way.

Do you mean the sorting done in "SelectionStreams Get" ?

With just a cursory look at the code, it seems we could get the language out of "m_SelectionStreams" at the beginning of "CDVDPlayer::OpenDefaultStreams" ... or is that already too late?

(Sorry if I'm asking stupid questions ... I've literally only looked at the code for 5 minutes)

An easier approach would be (as already described by someone before) a toggle setting to disable the
"best quality" selection and just go for the DEFAULT or the first stream.

I would prefer doing a proper fix rather than just working around the "Best codec" issue.

While I personally disagree with the "Best codec" list, somebody put it there for a reason, so they would presumably like a proper fix better.

@Montellese
Owner

The sorting is done in PredicateAudioPriority() and using std::stable_sort().

You can get the langauge of the primary stream at the beginning of OpenDefaultStreams but you would have to put it in a member variable to be available in PredicateAudioPriority() (which is not very nice).

I'm not sure if there is THE proper fix for handling "best codec" vs "first stream". People who like the "best quality" approach won't really care about an extra toggle setting to disable it. In case you still want to try and implement the "only look at streams matching the video language or the language of the first stream" you need to consider the case where you can't open a stream (which is very rare) so you still need to remember during your decision process that there are streams in another language which you might have to fall back to.

@Uxorious

Quick question ... was this tested in Windows or in Linux (or both).
Looking at improving the language detection, and GetAudioLanguage is returning a 2-character code ... meaning it doesn't work.

[Edit: Fix language code function name]

@Montellese
Owner

Mainly tested on win32 but I also ran a few tests on Linux because there is some win32-specific code needed for all the 2-char to 3-char conversions and back then it worked fine with the one file I had available with multiple audio streams. Will recheck later.

@Uxorious

I'm running the XBMCbuntu build, and I tried forcing the audio language to "French" in the UI.
When this was done, GetAudioLanguage returned "fr".

@Montellese
Owner

That's odd because when setting the audio language we run g_LangCodeExpander.ConvertToThreeCharCode() so there should be a 3-char code unless it fails. In that case it will fall back to the language code of your user interface language (which requires some platform specific code IIRC).

@Uxorious

Yes I looked at all that.
Since the full string is passed in, ConvertToThreeCharCode is passed "French".
This means it drops down to the last section (size > 3).
This results in the load of the French XML file.
And finally it returns the language code from that file directly.
The file only lists "fr".

@Montellese
Owner

Yeah that would mean that the problem is m_languageCodeGeneral which is exactly where there is platform-specific code required. I'll check later but probably it will require a call to g_LangCodeExpander.ConvertTwoToThreeCharCode on line 232 of LangInfo.cpp.

@Montellese
Owner

I just pushed e539456 and it works fine with that for me on Kubuntu.

@popcornmix

I believe returning false when all other tests are equal is unsafe as it breaks the strict weak ordering requirement of std::stable_sort. i.e. compare(A,B) == compare(B,A) which is not allowed.

See #3188 for similar code where memory corruption was the observed outcome.

returning something like (&lh < &rh) would ensure strict weak ordering.

This is incorrect. The predicate defines the less than operator, and it's perfectly reasonable to have A < B == B < A == false.

What's not allowed is A < B == B < A == true.

(Thus the issue with #3188, as it confused != with less than)

Collaborator

Ah yes, you are right. Returning false here is okay. Returning true would be wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 7, 2012
  1. @elupus @Montellese

    use a sorted vector to select playback stream

    elupus authored Montellese committed
  2. @Montellese

    add ConvertWindowsToGeneralCharCode to CLangCodeExpander to get the n…

    Montellese authored
    …on-win32-specific three char code from a win32-specific one
  3. @Montellese

    dvdplayer: try to find an audiostream that matches the preferred audi…

    Montellese authored
    …o language setting when starting a video
  4. @Montellese

    dvdplayer: try to find a subtitle that matches the preferred subtitle…

    Montellese authored
    … language setting when starting a video
This page is out of date. Refresh to see the latest.
View
20 language/English/strings.po
@@ -1030,7 +1030,13 @@ msgstr ""
msgid "No results found"
msgstr ""
-#empty strings from id 285 to 286
+#: id:285
+msgid "Preferred audio language"
+msgstr ""
+
+#: id:286
+msgid "Preferred subtitle language"
+msgstr ""
#: id:287
msgctxt "Auto context with id 287"
@@ -1117,7 +1123,17 @@ msgstr ""
msgid "Non-interleaved"
msgstr ""
-#empty strings from id 307 to 311
+#empty string with id 307
+
+#: id:308
+msgid "Original stream's language"
+msgstr ""
+
+#: id:309
+msgid "User Interface language"
+msgstr ""
+
+#empty strings from id 310 to 311
#: id:312
msgid "(0=auto)"
View
39 xbmc/LangInfo.cpp
@@ -25,9 +25,7 @@
#include "guilib/LocalizeStrings.h"
#include "utils/log.h"
#include "utils/XBMCTinyXML.h"
-#ifdef _WIN32
#include "utils/LangCodeExpander.h"
-#endif
using namespace std;
@@ -226,6 +224,11 @@ bool CLangInfo::Load(const CStdString& strFileName)
if (! g_LangCodeExpander.ConvertTwoToThreeCharCode(m_defaultRegion.m_strLangLocaleName, m_defaultRegion.m_strLangLocaleName, true))
m_defaultRegion.m_strLangLocaleName = "";
}
+
+ if (!g_LangCodeExpander.ConvertWindowsToGeneralCharCode(m_defaultRegion.m_strLangLocaleName, m_languageCodeGeneral))
+ m_languageCodeGeneral = "";
+#else
+ m_languageCodeGeneral = m_defaultRegion.m_strLangLocaleName;
#endif
const TiXmlNode *pCharSets = pRootElement->FirstChild("charsets");
@@ -360,6 +363,8 @@ void CLangInfo::SetDefaults()
// Set the default region, we may be unable to load langinfo.xml
m_currentRegion=&m_defaultRegion;
+
+ m_languageCodeGeneral = "eng";
}
CStdString CLangInfo::GetGuiCharSet() const
@@ -381,6 +386,36 @@ CStdString CLangInfo::GetSubtitleCharSet() const
return strCharSet;
}
+// three char language code (not win32 specific)
+const CStdString& CLangInfo::GetAudioLanguage() const
+{
+ if (!m_audioLanguage.empty())
+ return m_audioLanguage;
+
+ return m_languageCodeGeneral;
+}
+
+void CLangInfo::SetAudioLanguage(const CStdString &language)
+{
+ if (language.empty() || !g_LangCodeExpander.ConvertToThreeCharCode(m_audioLanguage, language))
+ m_audioLanguage.clear();
+}
+
+// three char language code (not win32 specific)
+const CStdString& CLangInfo::GetSubtitleLanguage() const
+{
+ if (!m_subtitleLanguage.empty())
+ return m_subtitleLanguage;
+
+ return m_languageCodeGeneral;
+}
+
+void CLangInfo::SetSubtitleLanguage(const CStdString &language)
+{
+ if (language.empty() || !g_LangCodeExpander.ConvertToThreeCharCode(m_subtitleLanguage, language))
+ m_subtitleLanguage.clear();
+}
+
// two character codes as defined in ISO639
const CStdString& CLangInfo::GetDVDMenuLanguage() const
{
View
21 xbmc/LangInfo.h
@@ -37,6 +37,22 @@ class CLangInfo
CStdString GetGuiCharSet() const;
CStdString GetSubtitleCharSet() const;
+ // three char language code (not win32 specific)
+ const CStdString& GetLanguageCode() const { return m_languageCodeGeneral; }
+
+ const CStdString& GetAudioLanguage() const;
+ // language can either be a two char language code as defined in ISO639
+ // or a three char language code
+ // or a language name in english (as used by XBMC)
+ void SetAudioLanguage(const CStdString &language);
+
+ // three char language code (not win32 specific)
+ const CStdString& GetSubtitleLanguage() const;
+ // language can either be a two char language code as defined in ISO639
+ // or a three char language code
+ // or a language name in english (as used by XBMC)
+ void SetSubtitleLanguage(const CStdString &language);
+
const CStdString& GetDVDMenuLanguage() const;
const CStdString& GetDVDAudioLanguage() const;
const CStdString& GetDVDSubtitleLanguage() const;
@@ -141,6 +157,11 @@ class CLangInfo
MAPREGIONS m_regions;
CRegion* m_currentRegion; // points to the current region
CRegion m_defaultRegion; // default, will be used if no region available via langinfo.xml
+
+ CStdString m_audioLanguage;
+ CStdString m_subtitleLanguage;
+ // this is the general (not win32-specific) three char language code
+ CStdString m_languageCodeGeneral;
};
View
275 xbmc/cores/dvdplayer/DVDPlayer.cpp
@@ -74,6 +74,7 @@
#include "dialogs/GUIDialogKaiToast.h"
#include "utils/StringUtils.h"
#include "Util.h"
+#include "LangInfo.h"
using namespace std;
@@ -116,6 +117,88 @@ SelectionStream& CSelectionStreams::Get(StreamType type, int index)
return m_invalid;
}
+std::vector<SelectionStream> CSelectionStreams::Get(StreamType type)
+{
+ std::vector<SelectionStream> streams;
+ int count = Count(type);
+ for(int index = 0; index < count; ++index){
+ streams.push_back(Get(type, index));
+ }
+ return streams;
+}
+
+#define PREDICATE_RETURN(lh, rh) \
+ do { \
+ if((lh) != (rh)) \
+ return (lh) > (rh); \
+ } while(0)
+
+static bool PredicateAudioPriority(const SelectionStream& lh, const SelectionStream& rh)
+{
+ PREDICATE_RETURN(lh.type_index == g_settings.m_currentVideoSettings.m_AudioStream
+ , rh.type_index == g_settings.m_currentVideoSettings.m_AudioStream);
+
+ if(!g_guiSettings.GetString("locale.audiolanguage").Equals("original"))
+ {
+ CStdString audio_language = g_langInfo.GetAudioLanguage();
+ PREDICATE_RETURN(audio_language.Equals(lh.language.c_str())
+ , audio_language.Equals(rh.language.c_str()));
+ }
+
+ PREDICATE_RETURN(lh.flags & CDemuxStream::FLAG_DEFAULT
+ , rh.flags & CDemuxStream::FLAG_DEFAULT);
+
+ PREDICATE_RETURN(lh.channels
+ , rh.channels);
+
+ PREDICATE_RETURN(StreamUtils::GetCodecPriority(lh.codec)
+ , StreamUtils::GetCodecPriority(rh.codec));
+ return false;
+}
+
+static bool PredicateSubtitlePriority(const SelectionStream& lh, const SelectionStream& rh)
+{
+ if(!g_settings.m_currentVideoSettings.m_SubtitleOn)
+ {
+ PREDICATE_RETURN(lh.flags & CDemuxStream::FLAG_FORCED
+ , rh.flags & CDemuxStream::FLAG_FORCED);
+ }
+
+ PREDICATE_RETURN(lh.type_index == g_settings.m_currentVideoSettings.m_SubtitleStream
+ , rh.type_index == g_settings.m_currentVideoSettings.m_SubtitleStream);
+
+ CStdString subtitle_language = g_langInfo.GetSubtitleLanguage();
+ if(!g_guiSettings.GetString("locale.subtitlelanguage").Equals("original"))
+ {
+ PREDICATE_RETURN((lh.source == STREAM_SOURCE_DEMUX_SUB || lh.source == STREAM_SOURCE_TEXT) && subtitle_language.Equals(lh.language.c_str())
+ , (rh.source == STREAM_SOURCE_DEMUX_SUB || rh.source == STREAM_SOURCE_TEXT) && subtitle_language.Equals(rh.language.c_str()));
+ }
+
+ PREDICATE_RETURN(lh.source == STREAM_SOURCE_DEMUX_SUB
+ , rh.source == STREAM_SOURCE_DEMUX_SUB);
+
+ PREDICATE_RETURN(lh.source == STREAM_SOURCE_TEXT
+ , rh.source == STREAM_SOURCE_TEXT);
+
+ if(!g_guiSettings.GetString("locale.subtitlelanguage").Equals("original"))
+ {
+ PREDICATE_RETURN(subtitle_language.Equals(lh.language.c_str())
+ , subtitle_language.Equals(rh.language.c_str()));
+ }
+
+ PREDICATE_RETURN(lh.flags & CDemuxStream::FLAG_DEFAULT
+ , rh.flags & CDemuxStream::FLAG_DEFAULT);
+
+ return false;
+}
+
+static bool PredicateVideoPriority(const SelectionStream& lh, const SelectionStream& rh)
+{
+ PREDICATE_RETURN(lh.flags & CDemuxStream::FLAG_DEFAULT
+ , rh.flags & CDemuxStream::FLAG_DEFAULT);
+ return false;
+}
+
bool CSelectionStreams::Get(StreamType type, CDemuxStream::EFlags flag, SelectionStream& out)
{
CSingleLock lock(m_section);
@@ -204,9 +287,16 @@ void CSelectionStreams::Update(SelectionStream& s)
CSingleLock lock(m_section);
int index = IndexOf(s.type, s.source, s.id);
if(index >= 0)
- Get(s.type, index) = s;
+ {
+ SelectionStream& o = Get(s.type, index);
+ s.type_index = o.type_index;
+ o = s;
+ }
else
+ {
+ s.type_index = Count(s.type);
m_Streams.push_back(s);
+ }
}
void CSelectionStreams::Update(CDVDInputStream* input, CDVDDemux* demuxer)
@@ -572,178 +662,59 @@ bool CDVDPlayer::OpenDemuxStream()
void CDVDPlayer::OpenDefaultStreams()
{
- int count;
+ SelectionStreams streams;
bool valid;
- bool force = false;
- SelectionStream st;
// open video stream
- count = m_SelectionStreams.Count(STREAM_VIDEO);
- valid = false;
-
- if(!valid
- && m_SelectionStreams.Get(STREAM_VIDEO, CDemuxStream::FLAG_DEFAULT, st))
+ streams = m_SelectionStreams.Get(STREAM_VIDEO, PredicateVideoPriority);
+ valid = false;
+ for(SelectionStreams::iterator it = streams.begin(); it != streams.end() && !valid; ++it)
{
- if(OpenVideoStream(st.id, st.source))
- valid = true;
- else
- CLog::Log(LOGWARNING, "%s - failed to open default stream (%d)", __FUNCTION__, st.id);
+ if(OpenVideoStream(it->id, it->source))
+ valid = true;;
}
+ if(!valid)
+ CloseVideoStream(true);
+
+ // open audio stream
+ if(m_PlayerOptions.video_only)
+ streams.clear();
+ else
+ streams = m_SelectionStreams.Get(STREAM_AUDIO, PredicateAudioPriority);
+ valid = false;
- for(int i = 0;i<count && !valid;i++)
+ for(SelectionStreams::iterator it = streams.begin(); it != streams.end() && !valid; ++it)
{
- SelectionStream& s = m_SelectionStreams.Get(STREAM_VIDEO, i);
- if(OpenVideoStream(s.id, s.source))
+ if(OpenAudioStream(it->id, it->source))
valid = true;
}
if(!valid)
- CloseVideoStream(true);
+ CloseAudioStream(true);
- if(!m_PlayerOptions.video_only)
- {
- // open audio stream
- count = m_SelectionStreams.Count(STREAM_AUDIO);
- valid = false;
- if(g_settings.m_currentVideoSettings.m_AudioStream >= 0
- && g_settings.m_currentVideoSettings.m_AudioStream < count)
- {
- SelectionStream& s = m_SelectionStreams.Get(STREAM_AUDIO, g_settings.m_currentVideoSettings.m_AudioStream);
- if(OpenAudioStream(s.id, s.source))
- valid = true;
- else
- CLog::Log(LOGWARNING, "%s - failed to restore selected audio stream (%d)", __FUNCTION__, g_settings.m_currentVideoSettings.m_AudioStream);
- }
-
- if(!valid
- && m_SelectionStreams.Get(STREAM_AUDIO, CDemuxStream::FLAG_DEFAULT, st))
- {
- if(OpenAudioStream(st.id, st.source))
- valid = true;
- else
- CLog::Log(LOGWARNING, "%s - failed to open default stream (%d)", __FUNCTION__, st.id);
- }
-
- if(!valid
- && count > 1)
- {
- /*
- * If there is more than one audio stream and a valid one is not chosen yet, select the one
- * with the maximum number of channels or the best codec if the same number of channels.
- */
- int max_channels = -1;
- int max_codec_priority = -1;
- CStdString max_codec;
- int max_stream_id = -1;
- for(int i = 0; i < count; i++)
- {
- SelectionStream& s = m_SelectionStreams.Get(STREAM_AUDIO, i);
- if(s.source == STREAM_SOURCE_DEMUX) // Only demux streams
- {
- int codec_priority = StreamUtils::GetCodecPriority(s.codec);
- if(s.channels > max_channels
- ||(s.channels == max_channels && codec_priority > max_codec_priority))
- {
- max_channels = s.channels;
- max_codec_priority = codec_priority;
- max_codec = s.codec;
- max_stream_id = i;
- }
- }
- }
- if(max_stream_id >= 0)
- {
- SelectionStream& s = m_SelectionStreams.Get(STREAM_AUDIO, max_stream_id);
- if(OpenAudioStream(s.id, s.source))
- {
- valid = true;
- CLog::Log(LOGDEBUG, "%s - using automatically selected audio stream (%d) based on codec '%s' and maximum number of channels '%d'",
- __FUNCTION__, s.id, max_codec.c_str(), max_channels);
- }
- else
- CLog::Log(LOGWARNING, "%s - failed to open automatically selected audio stream (%d)", __FUNCTION__, s.id);
- }
- }
-
- for(int i = 0; i<count && !valid; i++)
- {
- SelectionStream& s = m_SelectionStreams.Get(STREAM_AUDIO, i);
- if(OpenAudioStream(s.id, s.source))
- valid = true;
- }
- if(!valid)
- CloseAudioStream(true);
- }
+ // enable subtitles
+ m_dvdPlayerVideo.EnableSubtitle(g_settings.m_currentVideoSettings.m_SubtitleOn);
// open subtitle stream
- count = m_SelectionStreams.Count(STREAM_SUBTITLE);
- valid = false;
-
- // if subs are disabled, check for forced
- if(!valid && !g_settings.m_currentVideoSettings.m_SubtitleOn
- && m_SelectionStreams.Get(STREAM_SUBTITLE, CDemuxStream::FLAG_FORCED, st))
+ streams = m_SelectionStreams.Get(STREAM_SUBTITLE, PredicateSubtitlePriority);
+ valid = false;
+ for(SelectionStreams::iterator it = streams.begin(); it != streams.end() && !valid; ++it)
{
- if(OpenSubtitleStream(st.id, st.source))
+ if(OpenSubtitleStream(it->id, it->source))
{
valid = true;
- force = true;
+ if(it->flags & CDemuxStream::FLAG_FORCED)
+ m_dvdPlayerVideo.EnableSubtitle(true);
}
- else
- CLog::Log(LOGWARNING, "%s - failed to open default/forced stream (%d)", __FUNCTION__, st.id);
- }
-
- // restore selected
- if(!valid
- && g_settings.m_currentVideoSettings.m_SubtitleStream >= 0
- && g_settings.m_currentVideoSettings.m_SubtitleStream < count)
- {
- SelectionStream& s = m_SelectionStreams.Get(STREAM_SUBTITLE, g_settings.m_currentVideoSettings.m_SubtitleStream);
- if(OpenSubtitleStream(s.id, s.source))
- valid = true;
- else
- CLog::Log(LOGWARNING, "%s - failed to restore selected subtitle stream (%d)", __FUNCTION__, g_settings.m_currentVideoSettings.m_SubtitleStream);
- }
-
- // check if there are external subtitles available
- for(int i = 0;i<count && !valid; i++)
- {
- SelectionStream& s = m_SelectionStreams.Get(STREAM_SUBTITLE, i);
- if ((s.source == STREAM_SOURCE_DEMUX_SUB || s.source == STREAM_SOURCE_TEXT)
- && OpenSubtitleStream(s.id, s.source))
- valid = true;
- }
-
- // select default
- if(!valid
- && m_SelectionStreams.Get(STREAM_SUBTITLE, CDemuxStream::FLAG_DEFAULT, st))
- {
- if(OpenSubtitleStream(st.id, st.source))
- valid = true;
- else
- CLog::Log(LOGWARNING, "%s - failed to open default/forced stream (%d)", __FUNCTION__, st.id);
- }
-
- // select first
- for(int i = 0;i<count && !valid; i++)
- {
- SelectionStream& s = m_SelectionStreams.Get(STREAM_SUBTITLE, i);
- if(OpenSubtitleStream(s.id, s.source))
- valid = true;
}
if(!valid)
- CloseSubtitleStream(false);
-
- if(valid && (g_settings.m_currentVideoSettings.m_SubtitleOn || force) && !m_PlayerOptions.video_only)
- m_dvdPlayerVideo.EnableSubtitle(true);
- else
- m_dvdPlayerVideo.EnableSubtitle(false);
+ CloseSubtitleStream(true);
- // open teletext data stream
- count = m_SelectionStreams.Count(STREAM_TELETEXT);
- valid = false;
- for(int i = 0;i<count && !valid;i++)
+ // open teletext stream
+ streams = m_SelectionStreams.Get(STREAM_TELETEXT);
+ valid = false;
+ for(SelectionStreams::iterator it = streams.begin(); it != streams.end() && !valid; ++it)
{
- SelectionStream& s = m_SelectionStreams.Get(STREAM_TELETEXT, i);
- if(OpenTeletextStream(s.id, s.source))
+ if(OpenTeletextStream(it->id, it->source))
valid = true;
}
if(!valid)
View
11 xbmc/cores/dvdplayer/DVDPlayer.h
@@ -108,6 +108,7 @@ class CCurrentStream
typedef struct
{
StreamType type;
+ int type_index;
std::string filename;
std::string filename2; // for vobsub subtitles, 2 files are necessary (idx/sub)
std::string language;
@@ -119,6 +120,8 @@ typedef struct
int channels;
} SelectionStream;
+typedef std::vector<SelectionStream> SelectionStreams;
+
class CSelectionStreams
{
CCriticalSection m_section;
@@ -138,6 +141,14 @@ class CSelectionStreams
SelectionStream& Get (StreamType type, int index);
bool Get (StreamType type, CDemuxStream::EFlags flag, SelectionStream& out);
+ SelectionStreams Get(StreamType type);
+ template<typename Compare> SelectionStreams Get(StreamType type, Compare compare)
+ {
+ SelectionStreams streams = Get(type);
+ std::stable_sort(streams.begin(), streams.end(), compare);
+ return streams;
+ }
+
void Clear (StreamType type, StreamSource source);
int Source (StreamSource source, std::string filename);
View
15 xbmc/settings/GUISettings.cpp
@@ -845,6 +845,9 @@ void CGUISettings::Initialize()
AddBool(loc, "locale.timeserver", 168, false);
AddString(loc, "locale.timeserveraddress", 731, "pool.ntp.org", EDIT_CONTROL_INPUT);
#endif
+ AddSeparator(loc, "locale.sep3");
+ AddString(loc, "locale.audiolanguage", 285, "original", SPIN_CONTROL_TEXT);
+ AddString(loc, "locale.subtitlelanguage", 286, "original", SPIN_CONTROL_TEXT);
CSettingsCategory* fl = AddCategory(7, "filelists", 14081);
AddBool(fl, "filelists.showparentdiritems", 13306, true);
@@ -1263,6 +1266,18 @@ void CGUISettings::LoadXML(TiXmlElement *pRootElement, bool hideSettings /* = fa
g_timezone.SetTimezone(timezone);
}
#endif
+
+ CStdString streamLanguage = GetString("locale.audiolanguage");
+ if (!streamLanguage.Equals("original") && !streamLanguage.Equals("default"))
+ g_langInfo.SetAudioLanguage(streamLanguage);
+ else
+ g_langInfo.SetAudioLanguage("");
+
+ streamLanguage = GetString("locale.subtitlelanguage");
+ if (!streamLanguage.Equals("original") && !streamLanguage.Equals("default"))
+ g_langInfo.SetSubtitleLanguage(streamLanguage);
+ else
+ g_langInfo.SetSubtitleLanguage("");
}
void CGUISettings::LoadFromXML(TiXmlElement *pRootElement, mapIter &it, bool advanced /* = false */)
View
73 xbmc/settings/GUIWindowSettingsCategory.cpp
@@ -436,9 +436,22 @@ void CGUIWindowSettingsCategory::CreateSettings()
else if (strSetting.Equals("locale.language"))
{
AddSetting(pSetting, group->GetWidth(), iControlID);
+ GetSetting(pSetting->GetSetting())->SetDelayed();
FillInLanguages(pSetting);
continue;
}
+ else if (strSetting.Equals("locale.audiolanguage") || strSetting.Equals("locale.subtitlelanguage"))
+ {
+ AddSetting(pSetting, group->GetWidth(), iControlID);
+ vector<CStdString> languages;
+ languages.push_back(g_localizeStrings.Get(308));
+ languages.push_back(g_localizeStrings.Get(309));
+ vector<CStdString> languageKeys;
+ languageKeys.push_back("original");
+ languageKeys.push_back("default");
+ FillInLanguages(pSetting, languages, languageKeys);
+ continue;
+ }
#ifdef _LINUX
else if (strSetting.Equals("locale.timezonecountry"))
{
@@ -1447,6 +1460,52 @@ void CGUIWindowSettingsCategory::OnSettingChanged(CBaseSettingControl *pSettingC
if (g_graphicsContext.IsFullScreenRoot())
g_graphicsContext.SetVideoResolution(g_graphicsContext.GetVideoResolution(), true);
}
+ else if (strSetting.Equals("locale.audiolanguage"))
+ { // new audio language chosen...
+ CSettingString *pSettingString = (CSettingString *)pSettingControl->GetSetting();
+ CGUISpinControlEx *pControl = (CGUISpinControlEx *)GetControl(pSettingControl->GetID());
+ int iLanguage = pControl->GetValue();
+ if (iLanguage < 2)
+ {
+ if (iLanguage < 1)
+ g_guiSettings.SetString(strSetting, "original");
+ else
+ g_guiSettings.SetString(strSetting, "default");
+ g_langInfo.SetAudioLanguage("");
+ }
+ else
+ {
+ CStdString strLanguage = pControl->GetCurrentLabel();
+ if (strLanguage != pSettingString->GetData())
+ {
+ g_guiSettings.SetString(strSetting, strLanguage);
+ g_langInfo.SetAudioLanguage(strLanguage);
+ }
+ }
+ }
+ else if (strSetting.Equals("locale.subtitlelanguage"))
+ { // new subtitle language chosen...
+ CSettingString *pSettingString = (CSettingString *)pSettingControl->GetSetting();
+ CGUISpinControlEx *pControl = (CGUISpinControlEx *)GetControl(pSettingControl->GetID());
+ int iLanguage = pControl->GetValue();
+ if (iLanguage < 2)
+ {
+ if (iLanguage < 1)
+ g_guiSettings.SetString(strSetting, "original");
+ else
+ g_guiSettings.SetString(strSetting, "default");
+ g_langInfo.SetSubtitleLanguage("");
+ }
+ else
+ {
+ CStdString strLanguage = pControl->GetCurrentLabel();
+ if (strLanguage != pSettingString->GetData())
+ {
+ g_guiSettings.SetString(strSetting, strLanguage);
+ g_langInfo.SetSubtitleLanguage(strLanguage);
+ }
+ }
+ }
else if (strSetting.Equals("locale.language"))
{ // new language chosen...
CSettingString *pSettingString = (CSettingString *)pSettingControl->GetSetting();
@@ -2348,12 +2407,11 @@ void CGUIWindowSettingsCategory::OnRefreshRateChanged(RESOLUTION nextRes)
}
}
-void CGUIWindowSettingsCategory::FillInLanguages(CSetting *pSetting)
+void CGUIWindowSettingsCategory::FillInLanguages(CSetting *pSetting, const std::vector<CStdString> &languages /* = std::vector<CStdString>() */, const std::vector<CStdString> &languageKeys /* = std::vector<CStdString>() */)
{
CSettingString *pSettingString = (CSettingString *)pSetting;
CBaseSettingControl *setting = GetSetting(pSetting->GetSetting());
CGUISpinControlEx *pControl = (CGUISpinControlEx *)GetControl(setting->GetID());
- setting->SetDelayed();
pControl->Clear();
//find languages...
@@ -2361,7 +2419,6 @@ void CGUIWindowSettingsCategory::FillInLanguages(CSetting *pSetting)
CDirectory::GetDirectory("special://xbmc/language/", items);
int iCurrentLang = 0;
- int iLanguage = 0;
vector<CStdString> vecLanguage;
for (int i = 0; i < items.Size(); ++i)
{
@@ -2376,12 +2433,16 @@ void CGUIWindowSettingsCategory::FillInLanguages(CSetting *pSetting)
}
sort(vecLanguage.begin(), vecLanguage.end(), sortstringbyname());
+ // Add language options passed by parameter at the beginning
+ if (languages.size() > 0)
+ vecLanguage.insert(vecLanguage.begin(), languages.begin(), languages.begin() + languages.size());
for (unsigned int i = 0; i < vecLanguage.size(); ++i)
{
CStdString strLanguage = vecLanguage[i];
- if (strcmpi(strLanguage.c_str(), pSettingString->GetData().c_str()) == 0)
- iCurrentLang = iLanguage;
- pControl->AddLabel(strLanguage, iLanguage++);
+ if ((i < languageKeys.size() && strcmpi(languageKeys[i].c_str(), pSettingString->GetData().c_str()) == 0) ||
+ strcmpi(strLanguage.c_str(), pSettingString->GetData().c_str()) == 0)
+ iCurrentLang = i;
+ pControl->AddLabel(strLanguage, i);
}
pControl->SetValue(iCurrentLang);
View
2  xbmc/settings/GUIWindowSettingsCategory.h
@@ -48,7 +48,7 @@ class CGUIWindowSettingsCategory :
void FillInCharSets(CSetting *pSetting);
void FillInSkinFonts(CSetting *pSetting);
void FillInSoundSkins(CSetting *pSetting);
- void FillInLanguages(CSetting *pSetting);
+ void FillInLanguages(CSetting *pSetting, const std::vector<CStdString> &languages = std::vector<CStdString>(), const std::vector<CStdString> &languageKeys = std::vector<CStdString>());
DisplayMode FillInScreens(CStdString strSetting, RESOLUTION res);
void FillInResolutions(CStdString strSetting, DisplayMode mode, RESOLUTION res, bool UserChange);
void FillInRefreshRates(CStdString strSetting, RESOLUTION res, bool UserChange);
View
68 xbmc/utils/LangCodeExpander.cpp
@@ -21,6 +21,7 @@
#include "LangCodeExpander.h"
#include "utils/XBMCTinyXML.h"
+#include "LangInfo.h"
#include "utils/log.h"
#define MAKECODE(a, b, c, d) ((((long)(a))<<24) | (((long)(b))<<16) | (((long)(c))<<8) | (long)(d))
@@ -141,7 +142,7 @@ bool CLangCodeExpander::Lookup(CStdString& desc, const int code)
return Lookup(desc, lang);
}
-#ifdef _WIN32
+#ifdef TARGET_WINDOWS
bool CLangCodeExpander::ConvertTwoToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strTwoCharCode, bool localeHack /*= false*/)
#else
bool CLangCodeExpander::ConvertTwoToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strTwoCharCode)
@@ -175,6 +176,50 @@ bool CLangCodeExpander::ConvertTwoToThreeCharCode(CStdString& strThreeCharCode,
return false;
}
+#ifdef TARGET_WINDOWS
+bool CLangCodeExpander::ConvertToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strCharCode, bool localeHack /*= false*/)
+#else
+bool CLangCodeExpander::ConvertToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strCharCode)
+#endif
+{
+ if (strCharCode.size() == 2)
+#ifdef TARGET_WINDOWS
+ return g_LangCodeExpander.ConvertTwoToThreeCharCode(strThreeCharCode, strCharCode, localeHack);
+#else
+ return g_LangCodeExpander.ConvertTwoToThreeCharCode(strThreeCharCode, strCharCode);
+#endif
+ else if (strCharCode.size() == 3)
+ {
+ for (unsigned int index = 0; index < sizeof(CharCode2To3) / sizeof(CharCode2To3[0]); ++index)
+ {
+#ifdef TARGET_WINDOWS
+ if (strCharCode.Equals(CharCode2To3[index].id) ||
+ (localeHack && CharCode2To3[index].win_id != NULL && strCharCode.Equals(CharCode2To3[index].win_id)))
+#else
+ if (strCharCode.Equals(CharCode2To3[index].id))
+#endif
+ {
+ strThreeCharCode = strCharCode;
+ return true;
+ }
+ }
+ }
+ else if (strCharCode.size() > 3)
+ {
+ CStdString strLangInfoPath;
+ strLangInfoPath.Format("special://xbmc/language/%s/langinfo.xml", strCharCode.c_str());
+ CLangInfo langInfo;
+ if (!langInfo.Load(strLangInfoPath))
+ return false;
+
+ strThreeCharCode = langInfo.GetLanguageCode();
+ return true;
+ }
+
+ return false;
+}
+
+#ifdef TARGET_WINDOWS
bool CLangCodeExpander::ConvertLinuxToWindowsRegionCodes(const CStdString& strTwoCharCode, CStdString& strThreeCharCode)
{
if (strTwoCharCode.length() != 2)
@@ -196,6 +241,27 @@ bool CLangCodeExpander::ConvertLinuxToWindowsRegionCodes(const CStdString& strTw
return true;
}
+bool CLangCodeExpander::ConvertWindowsToGeneralCharCode(const CStdString& strWindowsCharCode, CStdString& strThreeCharCode)
+{
+ if (strWindowsCharCode.length() != 3)
+ return false;
+
+ CStdString strLower(strWindowsCharCode);
+ strLower.MakeLower();
+ for (unsigned int index = 0; index < sizeof(CharCode2To3) / sizeof(CharCode2To3[0]); ++index)
+ {
+ if ((CharCode2To3[index].win_id && strLower.Equals(CharCode2To3[index].win_id)) ||
+ strLower.Equals(CharCode2To3[index].id))
+ {
+ strThreeCharCode = CharCode2To3[index].id;
+ return true;
+ }
+ }
+
+ return true;
+}
+#endif
+
bool CLangCodeExpander::LookupInMap(CStdString& desc, const CStdString& code)
{
STRINGLOOKUPTABLE::iterator it;
View
8 xbmc/utils/LangCodeExpander.h
@@ -34,12 +34,18 @@ class CLangCodeExpander
bool Lookup(CStdString& desc, const CStdString& code);
bool Lookup(CStdString& desc, const int code);
-#ifdef _WIN32
+#ifdef TARGET_WINDOWS
bool ConvertTwoToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strTwoCharCode, bool localeHack = false);
+ bool ConvertToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strCharCode, bool localeHack = false);
#else
bool ConvertTwoToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strTwoCharCode);
+ bool ConvertToThreeCharCode(CStdString& strThreeCharCode, const CStdString& strCharCode);
#endif
+
+#ifdef TARGET_WINDOWS
bool ConvertLinuxToWindowsRegionCodes(const CStdString& strTwoCharCode, CStdString& strThreeCharCode);
+ bool ConvertWindowsToGeneralCharCode(const CStdString& strWindowsCharCode, CStdString& strThreeCharCode);
+#endif
void LoadUserCodes(const TiXmlElement* pRootElement);
void Clear();
Something went wrong with that request. Please try again.