From 32398da5c47a1cccafc8d510c196164060710bab Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Wed, 4 Apr 2018 18:27:57 -0700 Subject: [PATCH] Fix default_language not effective with 2-char code Two-character ISO-639 code in --default_language was ignored due to a bug in language code matching as the language code in stream is always converted to 3-character code. Fixes #371. Change-Id: I8618938af583a417446636ff9efe1c72ce822c33 --- packager/app/test/packager_test.py | 68 ++++++++++++++++++- .../testdata/bear-640x360-av-por-golden.mpd | 3 +- .../bear-640x360-av-por-master-golden.m3u8 | 7 ++ packager/hls/base/master_playlist.cc | 4 +- packager/hls/base/master_playlist_unittest.cc | 4 +- packager/hls/base/media_playlist.cc | 36 ++++++---- packager/hls/base/media_playlist.h | 12 ++-- packager/hls/base/media_playlist_unittest.cc | 6 +- packager/hls/base/mock_media_playlist.h | 1 - packager/mpd/base/adaptation_set.cc | 3 +- packager/mpd/base/adaptation_set_unittest.cc | 8 --- packager/mpd/base/mpd_utils.cc | 3 +- packager/mpd/base/mpd_utils.h | 2 +- packager/packager.cc | 7 ++ 14 files changed, 122 insertions(+), 42 deletions(-) create mode 100644 packager/app/test/testdata/bear-640x360-av-por-master-golden.m3u8 diff --git a/packager/app/test/packager_test.py b/packager/app/test/packager_test.py index 2273b49ad9..6535cf2178 100755 --- a/packager/app/test/packager_test.py +++ b/packager/app/test/packager_test.py @@ -158,6 +158,7 @@ class PackagerAppTest(unittest.TestCase): time_shift_buffer_depth=0.0, generate_static_mpd=False, ad_cues=None, + default_language=None, use_fake_clock=True): flags = [] @@ -230,6 +231,9 @@ class PackagerAppTest(unittest.TestCase): if ad_cues: flags += ['--ad_cues', ad_cues] + if default_language: + flags += ['--default_language', default_language] + flags.append('--segment_duration=1') # Use fake clock, so output can be compared. if use_fake_clock: @@ -417,8 +421,32 @@ class PackagerFunctionalTest(PackagerAppTest): def testPackageAudioVideoWithLanguageOverride(self): self.assertPackageSuccess( - self._GetStreams(['audio', 'video'], language_override='por-BR'), - self._GetFlags()) + self._GetStreams(['audio', 'video'], language_override='por'), + self._GetFlags(default_language='por')) + self._DiffGold(self.output[0], 'bear-640x360-a-por-golden.mp4') + self._DiffGold(self.output[1], 'bear-640x360-v-golden.mp4') + self._DiffGold(self.mpd_output, 'bear-640x360-av-por-golden.mpd') + + def testPackageAudioVideoWithLanguageOverrideUsingMixingCode(self): + self.assertPackageSuccess( + self._GetStreams(['audio', 'video'], language_override='por'), + self._GetFlags(default_language='pt')) + self._DiffGold(self.output[0], 'bear-640x360-a-por-golden.mp4') + self._DiffGold(self.output[1], 'bear-640x360-v-golden.mp4') + self._DiffGold(self.mpd_output, 'bear-640x360-av-por-golden.mpd') + + def testPackageAudioVideoWithLanguageOverrideUsingMixingCode2(self): + self.assertPackageSuccess( + self._GetStreams(['audio', 'video'], language_override='pt'), + self._GetFlags(default_language='por')) + self._DiffGold(self.output[0], 'bear-640x360-a-por-golden.mp4') + self._DiffGold(self.output[1], 'bear-640x360-v-golden.mp4') + self._DiffGold(self.mpd_output, 'bear-640x360-av-por-golden.mpd') + + def testPackageAudioVideoWithLanguageOverrideUsingTwoCharacterCode(self): + self.assertPackageSuccess( + self._GetStreams(['audio', 'video'], language_override='pt'), + self._GetFlags(default_language='pt')) self._DiffGold(self.output[0], 'bear-640x360-a-por-golden.mp4') self._DiffGold(self.output[1], 'bear-640x360-v-golden.mp4') self._DiffGold(self.mpd_output, 'bear-640x360-av-por-golden.mpd') @@ -476,6 +504,42 @@ class PackagerFunctionalTest(PackagerAppTest): self._DiffGold( os.path.join(self.tmp_dir, 'video.m3u8'), 'bear-640x360-v-golden.m3u8') + def testPackageAvcAacTsLanguage(self): + # Currently we only support live packaging for ts. + self.assertPackageSuccess( + self._GetStreams( + ['audio', 'video'], + output_format='ts', + live=True, + hls=True, + language_override='por', + test_files=['bear-640x360.ts']), + self._GetFlags(output_hls=True, default_language='por')) + self._DiffGold(self.hls_master_playlist_output, + 'bear-640x360-av-por-master-golden.m3u8') + self._DiffGold( + os.path.join(self.tmp_dir, 'audio.m3u8'), 'bear-640x360-a-golden.m3u8') + self._DiffGold( + os.path.join(self.tmp_dir, 'video.m3u8'), 'bear-640x360-v-golden.m3u8') + + def testPackageAvcAacTsLanguageUsingTwoCharacterCode(self): + # Currently we only support live packaging for ts. + self.assertPackageSuccess( + self._GetStreams( + ['audio', 'video'], + output_format='ts', + live=True, + hls=True, + language_override='pt', + test_files=['bear-640x360.ts']), + self._GetFlags(output_hls=True, default_language='pt')) + self._DiffGold(self.hls_master_playlist_output, + 'bear-640x360-av-por-master-golden.m3u8') + self._DiffGold( + os.path.join(self.tmp_dir, 'audio.m3u8'), 'bear-640x360-a-golden.m3u8') + self._DiffGold( + os.path.join(self.tmp_dir, 'video.m3u8'), 'bear-640x360-v-golden.m3u8') + def testPackageAvcAc3Ts(self): # Currently we only support live packaging for ts. self.assertPackageSuccess( diff --git a/packager/app/test/testdata/bear-640x360-av-por-golden.mpd b/packager/app/test/testdata/bear-640x360-av-por-golden.mpd index 1a9fb6efcf..192314f03e 100644 --- a/packager/app/test/testdata/bear-640x360-av-por-golden.mpd +++ b/packager/app/test/testdata/bear-640x360-av-por-golden.mpd @@ -10,7 +10,8 @@ - + + output_audio.mp4 diff --git a/packager/app/test/testdata/bear-640x360-av-por-master-golden.m3u8 b/packager/app/test/testdata/bear-640x360-av-por-master-golden.m3u8 new file mode 100644 index 0000000000..c8876a0d18 --- /dev/null +++ b/packager/app/test/testdata/bear-640x360-av-por-master-golden.m3u8 @@ -0,0 +1,7 @@ +#EXTM3U +## Generated with https://github.com/google/shaka-packager version -- + +#EXT-X-MEDIA:TYPE=AUDIO,URI="audio.m3u8",GROUP-ID="default-audio-group",LANGUAGE="pt",NAME="stream_0",DEFAULT=YES,AUTOSELECT=YES,CHANNELS="2" + +#EXT-X-STREAM-INF:BANDWIDTH=1217518,CODECS="avc1.64001e,mp4a.40.2",RESOLUTION=640x360,AUDIO="default-audio-group" +video.m3u8 diff --git a/packager/hls/base/master_playlist.cc b/packager/hls/base/master_playlist.cc index 27224dca17..329a126904 100644 --- a/packager/hls/base/master_playlist.cc +++ b/packager/hls/base/master_playlist.cc @@ -231,7 +231,7 @@ void BuildMediaTag(const MediaPlaylist& playlist, tag.AddQuotedString("URI", base_url + playlist.file_name()); tag.AddQuotedString("GROUP-ID", group_id); - const std::string& language = playlist.GetLanguage(); + const std::string& language = playlist.language(); if (!language.empty()) { tag.AddQuotedString("LANGUAGE", language); } @@ -282,7 +282,7 @@ void BuildMediaTags( bool is_default = false; bool is_autoselect = false; - const std::string language = playlist->GetLanguage(); + const std::string language = playlist->language(); if (languages.find(language) == languages.end()) { is_default = !language.empty() && language == default_language; is_autoselect = true; diff --git a/packager/hls/base/master_playlist_unittest.cc b/packager/hls/base/master_playlist_unittest.cc index 748fe3cad3..e798e9ffae 100644 --- a/packager/hls/base/master_playlist_unittest.cc +++ b/packager/hls/base/master_playlist_unittest.cc @@ -78,12 +78,12 @@ std::unique_ptr CreateAudioPlaylist( std::unique_ptr playlist( new MockMediaPlaylist(kVodPlaylist, filename, name, group)); - EXPECT_CALL(*playlist, GetLanguage()).WillRepeatedly(Return(language)); EXPECT_CALL(*playlist, GetNumChannels()).WillRepeatedly(Return(channels)); playlist->SetStreamTypeForTesting( MediaPlaylist::MediaPlaylistStreamType::kAudio); playlist->SetCodecForTesting(codec); + playlist->SetLanguageForTesting(language); EXPECT_CALL(*playlist, Bitrate()) .Times(AtLeast(1)) @@ -101,9 +101,9 @@ std::unique_ptr CreateTextPlaylist( std::unique_ptr playlist( new MockMediaPlaylist(kVodPlaylist, filename, name, group)); - EXPECT_CALL(*playlist, GetLanguage()).WillRepeatedly(Return(language)); playlist->SetStreamTypeForTesting( MediaPlaylist::MediaPlaylistStreamType::kSubtitle); + playlist->SetLanguageForTesting(language); return playlist; } diff --git a/packager/hls/base/media_playlist.cc b/packager/hls/base/media_playlist.cc index ffddeaeca3..9f28cda560 100644 --- a/packager/hls/base/media_playlist.cc +++ b/packager/hls/base/media_playlist.cc @@ -36,6 +36,22 @@ uint32_t GetTimeScale(const MediaInfo& media_info) { return 0u; } +// Duplicated from MpdUtils because: +// 1. MpdUtils header depends on libxml header, which is not in the deps here +// 2. GetLanguage depends on MediaInfo from packager/mpd/ +// 3. Moving GetLanguage to LanguageUtils would create a a media => mpd dep. +// TODO(https://github.com/google/shaka-packager/issues/373): Fix this +// dependency situation and factor this out to a common location. +std::string GetLanguage(const MediaInfo& media_info) { + std::string lang; + if (media_info.has_audio_info()) { + lang = media_info.audio_info().language(); + } else if (media_info.has_text_info()) { + lang = media_info.text_info().language(); + } + return LanguageToShortestForm(lang); +} + void AppendExtXMap(const MediaInfo& media_info, std::string* out) { if (media_info.has_init_segment_name()) { Tag tag("#EXT-X-MAP", out); @@ -327,6 +343,10 @@ void MediaPlaylist::SetCodecForTesting(const std::string& codec) { codec_ = codec; } +void MediaPlaylist::SetLanguageForTesting(const std::string& language) { + language_ = language; +} + bool MediaPlaylist::SetMediaInfo(const MediaInfo& media_info) { const uint32_t time_scale = GetTimeScale(media_info); if (time_scale == 0) { @@ -347,6 +367,7 @@ bool MediaPlaylist::SetMediaInfo(const MediaInfo& media_info) { time_scale_ = time_scale; media_info_ = media_info; + language_ = GetLanguage(media_info); use_byte_range_ = !media_info_.has_segment_template(); return true; } @@ -473,21 +494,6 @@ void MediaPlaylist::SetTargetDuration(uint32_t target_duration) { target_duration_set_ = true; } -// Duplicated from MpdUtils because: -// 1. MpdUtils header depends on libxml header, which is not in the deps here -// 2. GetLanguage depends on MediaInfo from packager/mpd/ -// 3. Moving GetLanguage to LanguageUtils would create a a media => mpd dep. -// TODO: fix this dependency situation and factor this out to a common location -std::string MediaPlaylist::GetLanguage() const { - std::string lang; - if (media_info_.has_audio_info()) { - lang = media_info_.audio_info().language(); - } else if (media_info_.has_text_info()) { - lang = media_info_.text_info().language(); - } - return LanguageToShortestForm(lang); -} - int MediaPlaylist::GetNumChannels() const { return media_info_.audio_info().num_channels(); } diff --git a/packager/hls/base/media_playlist.h b/packager/hls/base/media_playlist.h index 2f50cd1564..20cf068748 100644 --- a/packager/hls/base/media_playlist.h +++ b/packager/hls/base/media_playlist.h @@ -86,6 +86,9 @@ class MediaPlaylist { /// For testing only. void SetCodecForTesting(const std::string& codec); + /// For testing only. + void SetLanguageForTesting(const std::string& language); + /// This must succeed before calling any other public methods. /// @param media_info is the info of the segments that are going to be added /// to this playlist. @@ -167,10 +170,6 @@ class MediaPlaylist { /// @param target_duration is the target duration for this playlist. virtual void SetTargetDuration(uint32_t target_duration); - /// @return the language of the media, as an ISO language tag in its shortest - /// form. May be an empty string for video. - virtual std::string GetLanguage() const; - /// @return number of channels for audio. 0 is returned for video. virtual int GetNumChannels() const; @@ -178,6 +177,10 @@ class MediaPlaylist { /// resolution values. virtual bool GetDisplayResolution(uint32_t* width, uint32_t* height) const; + /// @return the language of the media, as an ISO language tag in its shortest + /// form. May be an empty string for video. + std::string language() const { return language_; } + private: // Add a SegmentInfoEntry (#EXTINF). void AddSegmentInfoEntry(const std::string& segment_file_name, @@ -200,6 +203,7 @@ class MediaPlaylist { // Whether to use byte range for SegmentInfoEntry. bool use_byte_range_ = false; std::string codec_; + std::string language_; int media_sequence_number_ = 0; bool inserted_discontinuity_tag_ = false; int discontinuity_sequence_number_ = 0; diff --git a/packager/hls/base/media_playlist_unittest.cc b/packager/hls/base/media_playlist_unittest.cc index a481a57c76..0535c54855 100644 --- a/packager/hls/base/media_playlist_unittest.cc +++ b/packager/hls/base/media_playlist_unittest.cc @@ -449,15 +449,15 @@ TEST_F(MediaPlaylistMultiSegmentTest, GetLanguage) { // Check conversions from long to short form. media_info.mutable_audio_info()->set_language("eng"); ASSERT_TRUE(media_playlist_.SetMediaInfo(media_info)); - EXPECT_EQ("en", media_playlist_.GetLanguage()); // short form + EXPECT_EQ("en", media_playlist_.language()); // short form media_info.mutable_audio_info()->set_language("eng-US"); ASSERT_TRUE(media_playlist_.SetMediaInfo(media_info)); - EXPECT_EQ("en-US", media_playlist_.GetLanguage()); // region preserved + EXPECT_EQ("en-US", media_playlist_.language()); // region preserved media_info.mutable_audio_info()->set_language("apa"); ASSERT_TRUE(media_playlist_.SetMediaInfo(media_info)); - EXPECT_EQ("apa", media_playlist_.GetLanguage()); // no short form exists + EXPECT_EQ("apa", media_playlist_.language()); // no short form exists } TEST_F(MediaPlaylistMultiSegmentTest, GetNumChannels) { diff --git a/packager/hls/base/mock_media_playlist.h b/packager/hls/base/mock_media_playlist.h index 09ccc3ac5b..e46e8b500a 100644 --- a/packager/hls/base/mock_media_playlist.h +++ b/packager/hls/base/mock_media_playlist.h @@ -47,7 +47,6 @@ class MockMediaPlaylist : public MediaPlaylist { MOCK_CONST_METHOD0(Bitrate, uint64_t()); MOCK_CONST_METHOD0(GetLongestSegmentDuration, double()); MOCK_METHOD1(SetTargetDuration, void(uint32_t target_duration)); - MOCK_CONST_METHOD0(GetLanguage, std::string()); MOCK_CONST_METHOD0(GetNumChannels, int()); MOCK_CONST_METHOD2(GetDisplayResolution, bool(uint32_t* width, uint32_t* height)); diff --git a/packager/mpd/base/adaptation_set.cc b/packager/mpd/base/adaptation_set.cc index b4f755c228..fbc81e22b8 100644 --- a/packager/mpd/base/adaptation_set.cc +++ b/packager/mpd/base/adaptation_set.cc @@ -10,7 +10,6 @@ #include "packager/base/logging.h" #include "packager/base/strings/string_number_conversions.h" -#include "packager/media/base/language_utils.h" #include "packager/mpd/base/media_info.pb.h" #include "packager/mpd/base/mpd_options.h" #include "packager/mpd/base/mpd_utils.h" @@ -248,7 +247,7 @@ xml::scoped_xml_ptr AdaptationSet::GetXml() { adaptation_set.SetId(id_); adaptation_set.SetStringAttribute("contentType", content_type_); if (!lang_.empty() && lang_ != "und") { - adaptation_set.SetStringAttribute("lang", LanguageToShortestForm(lang_)); + adaptation_set.SetStringAttribute("lang", lang_); } // Note that std::{set,map} are ordered, so the last element is the max value. diff --git a/packager/mpd/base/adaptation_set_unittest.cc b/packager/mpd/base/adaptation_set_unittest.cc index 13bf8a5535..a91af411c8 100644 --- a/packager/mpd/base/adaptation_set_unittest.cc +++ b/packager/mpd/base/adaptation_set_unittest.cc @@ -148,14 +148,6 @@ TEST_F(AdaptationSetTest, CheckLanguageAttributeSet) { EXPECT_THAT(adaptation_set->GetXml().get(), AttributeEqual("lang", "en")); } -// Verify that language tags with subtags can still be converted. -TEST_F(AdaptationSetTest, CheckConvertLanguageWithSubtag) { - // "por-BR" is the long tag for Brazillian Portuguese. The short tag - // is "pt-BR", which is what should appear in the manifest. - auto adaptation_set = CreateAdaptationSet(kAnyAdaptationSetId, "por-BR"); - EXPECT_THAT(adaptation_set->GetXml().get(), AttributeEqual("lang", "pt-BR")); -} - TEST_F(AdaptationSetTest, CheckAdaptationSetId) { const uint32_t kAdaptationSetId = 42; auto adaptation_set = CreateAdaptationSet(kAdaptationSetId, kNoLanguage); diff --git a/packager/mpd/base/mpd_utils.cc b/packager/mpd/base/mpd_utils.cc index 049dbd7739..2402df3581 100644 --- a/packager/mpd/base/mpd_utils.cc +++ b/packager/mpd/base/mpd_utils.cc @@ -12,6 +12,7 @@ #include "packager/base/logging.h" #include "packager/base/strings/string_number_conversions.h" #include "packager/base/strings/string_util.h" +#include "packager/media/base/language_utils.h" #include "packager/mpd/base/adaptation_set.h" #include "packager/mpd/base/content_protection_element.h" #include "packager/mpd/base/representation.h" @@ -79,7 +80,7 @@ std::string GetLanguage(const MediaInfo& media_info) { } else if (media_info.has_text_info()) { lang = media_info.text_info().language(); } - return lang; + return LanguageToShortestForm(lang); } std::string GetCodecs(const MediaInfo& media_info) { diff --git a/packager/mpd/base/mpd_utils.h b/packager/mpd/base/mpd_utils.h index 02072f1297..557d138ce6 100644 --- a/packager/mpd/base/mpd_utils.h +++ b/packager/mpd/base/mpd_utils.h @@ -35,7 +35,7 @@ bool HasLiveOnlyFields(const MediaInfo& media_info); void RemoveDuplicateAttributes( ContentProtectionElement* content_protection_element); -// Returns a language tag. May be blank for video. +// Returns a language in ISO-639 shortest form. May be blank for video. std::string GetLanguage(const MediaInfo& media_info); // Returns a 'codecs' string that has all the video and audio codecs joined with diff --git a/packager/packager.cc b/packager/packager.cc index 2a324fc481..0bf14280df 100644 --- a/packager/packager.cc +++ b/packager/packager.cc @@ -839,6 +839,13 @@ Status Packager::Initialize( hls_params.master_playlist_output = File::MakeCallbackFileName( internal->buffer_callback_params, hls_params.master_playlist_output); } + // Both DASH and HLS require language to follow RFC5646 + // (https://tools.ietf.org/html/rfc5646), which requires the language to be + // in the shortest form. + mpd_params.default_language = + LanguageToShortestForm(mpd_params.default_language); + hls_params.default_language = + LanguageToShortestForm(hls_params.default_language); if (!mpd_params.mpd_output.empty()) { const bool on_demand_dash_profile =