From f089d1d0d474139397f2a62f734fbdd33af6963b Mon Sep 17 00:00:00 2001 From: KongQun Yang Date: Wed, 13 Jun 2018 16:50:16 -0700 Subject: [PATCH] Deprecate --mp4_use_decoding_timestamp_in_timeline It was implemented to workaround Chromium's DTS https://crbug.com/398130, but the workaround does not really work in all situations. Remove it now as we already have another workaround available. Change-Id: I291f559d78120fb743a6679b7d927e5bbc5b6b4e --- docs/source/options/mp4_output_options.rst | 5 +---- packager/app/muxer_flags.cc | 6 ------ packager/app/muxer_flags.h | 1 - packager/app/packager_main.cc | 8 -------- packager/app/retired_flags.cc | 11 +++++++++++ packager/app/retired_flags.h | 1 + packager/media/formats/mp4/fragmenter.cc | 3 +-- packager/media/formats/mp4/fragmenter.h | 10 ---------- packager/media/formats/mp4/segmenter.cc | 4 ---- packager/media/public/mp4_output_params.h | 5 ----- 10 files changed, 14 insertions(+), 40 deletions(-) diff --git a/docs/source/options/mp4_output_options.rst b/docs/source/options/mp4_output_options.rst index 7b9ad64715..b8cc84da84 100644 --- a/docs/source/options/mp4_output_options.rst +++ b/docs/source/options/mp4_output_options.rst @@ -7,10 +7,7 @@ MP4 output options --mp4_use_decoding_timestamp_in_timeline - If set, decoding timestamp instead of presentation timestamp will be used - when generating media timeline, e.g. timestamps in sidx and mpd. This is - to workaround a Chromium bug that decoding timestamp is used in buffered - range, https://crbug.com/398130. Default false. + Deprecated. Do not use. --num_subsegments_per_sidx diff --git a/packager/app/muxer_flags.cc b/packager/app/muxer_flags.cc index a18a4e7754..4c31fbdb00 100644 --- a/packager/app/muxer_flags.cc +++ b/packager/app/muxer_flags.cc @@ -44,9 +44,3 @@ DEFINE_string(temp_dir, DEFINE_bool(mp4_include_pssh_in_stream, true, "MP4 only: include pssh in the encrypted stream."); -DEFINE_bool(mp4_use_decoding_timestamp_in_timeline, - false, - "If set, decoding timestamp instead of presentation timestamp will " - "be used when generating media timeline, e.g. timestamps in sidx " - "and mpd. This is to workaround a Chromium bug that decoding " - "timestamp is used in buffered range, https://crbug.com/398130."); diff --git a/packager/app/muxer_flags.h b/packager/app/muxer_flags.h index a88a777a3d..608529a351 100644 --- a/packager/app/muxer_flags.h +++ b/packager/app/muxer_flags.h @@ -19,6 +19,5 @@ DECLARE_bool(fragment_sap_aligned); DECLARE_int32(num_subsegments_per_sidx); DECLARE_string(temp_dir); DECLARE_bool(mp4_include_pssh_in_stream); -DECLARE_bool(mp4_use_decoding_timestamp_in_timeline); #endif // APP_MUXER_FLAGS_H_ diff --git a/packager/app/packager_main.cc b/packager/app/packager_main.cc index 3f65302f72..f04c235987 100644 --- a/packager/app/packager_main.cc +++ b/packager/app/packager_main.cc @@ -378,14 +378,6 @@ base::Optional GetPackagingParams() { Mp4OutputParams& mp4_params = packaging_params.mp4_output_params; mp4_params.num_subsegments_per_sidx = FLAGS_num_subsegments_per_sidx; - if (FLAGS_mp4_use_decoding_timestamp_in_timeline) { - LOG(WARNING) << "Flag --mp4_use_decoding_timestamp_in_timeline is set. " - "Note that it is a temporary hack to workaround Chromium " - "bug https://crbug.com/398130. The flag may be removed " - "when the Chromium bug is fixed."; - } - mp4_params.use_decoding_timestamp_in_timeline = - FLAGS_mp4_use_decoding_timestamp_in_timeline; mp4_params.include_pssh_in_stream = FLAGS_mp4_include_pssh_in_stream; packaging_params.output_media_info = FLAGS_output_media_info; diff --git a/packager/app/retired_flags.cc b/packager/app/retired_flags.cc index 1c626cbb15..8b6c80d2c3 100644 --- a/packager/app/retired_flags.cc +++ b/packager/app/retired_flags.cc @@ -28,6 +28,9 @@ DEFINE_string(playready_key, "", "This flag is deprecated. Use --enable_raw_key_encryption with " "--generate_playready_pssh to generate PlayReady PSSH."); +DEFINE_bool(mp4_use_decoding_timestamp_in_timeline, + false, + "This flag is deprecated. Do not use."); // The current gflags library does not provide a way to check whether a flag is // set in command line. If a flag has a different value to its default value, @@ -45,6 +48,12 @@ bool InformRetiredDefaultTrueFlag(const char* flagname, bool value) { return true; } +bool InformRetiredDefaultFalseFlag(const char* flagname, bool value) { + if (value) + fprintf(stderr, "WARNING: %s is deprecated and ignored.\n", flagname); + return true; +} + bool InformRetiredDefaultDoubleFlag(const char* flagname, double value) { if (value != 0) fprintf(stderr, "WARNING: %s is deprecated and ignored.\n", flagname); @@ -57,3 +66,5 @@ DEFINE_validator(webm_subsample_encryption, &InformRetiredDefaultTrueFlag); DEFINE_validator(availability_time_offset, &InformRetiredDefaultDoubleFlag); DEFINE_validator(playready_key_id, &InformRetiredStringFlag); DEFINE_validator(playready_key, &InformRetiredStringFlag); +DEFINE_validator(mp4_use_decoding_timestamp_in_timeline, + &InformRetiredDefaultFalseFlag); diff --git a/packager/app/retired_flags.h b/packager/app/retired_flags.h index e45ac97cfd..bbe235f670 100644 --- a/packager/app/retired_flags.h +++ b/packager/app/retired_flags.h @@ -12,3 +12,4 @@ DECLARE_bool(webm_subsample_encryption); DECLARE_double(availability_time_offset); DECLARE_string(playready_key_id); DECLARE_string(playready_key); +DECLARE_bool(mp4_use_decoding_timestamp_in_timeline); diff --git a/packager/media/formats/mp4/fragmenter.cc b/packager/media/formats/mp4/fragmenter.cc index 3654b6d8fa..1295124eb4 100644 --- a/packager/media/formats/mp4/fragmenter.cc +++ b/packager/media/formats/mp4/fragmenter.cc @@ -49,7 +49,6 @@ void NewSampleEncryptionEntry(const DecryptConfig& decrypt_config, Fragmenter::Fragmenter(std::shared_ptr stream_info, TrackFragment* traf) : stream_info_(std::move(stream_info)), - use_decoding_timestamp_in_timeline_(false), traf_(traf), seek_preroll_(GetSeekPreroll(*stream_info_)), fragment_initialized_(false), @@ -103,7 +102,7 @@ Status Fragmenter::AddSample(const MediaSample& sample) { const int64_t pts = sample.pts(); const int64_t dts = sample.dts(); - const int64_t timestamp = use_decoding_timestamp_in_timeline_ ? dts : pts; + const int64_t timestamp = pts; // Set |earliest_presentation_time_| to |timestamp| if |timestamp| is smaller // or if it is not yet initialized (kInvalidTime > timestamp is always true). if (earliest_presentation_time_ > timestamp) diff --git a/packager/media/formats/mp4/fragmenter.h b/packager/media/formats/mp4/fragmenter.h index 799d9396ba..16daeb102f 100644 --- a/packager/media/formats/mp4/fragmenter.h +++ b/packager/media/formats/mp4/fragmenter.h @@ -67,15 +67,6 @@ class Fragmenter { return key_frame_infos_; } - /// Set the flag use_decoding_timestamp_in_timeline, which if set to true, use - /// decoding timestamp instead of presentation timestamp in media timeline, - /// which is needed to workaround a Chromium bug that decoding timestamp is - /// used in buffered range, https://crbug.com/398130. - void set_use_decoding_timestamp_in_timeline( - bool use_decoding_timestamp_in_timeline) { - use_decoding_timestamp_in_timeline_ = use_decoding_timestamp_in_timeline; - } - /// Set the flag allow_use_adjust_earliest_presentation_time, which if set to /// true, earlist_presentation_time (EPT) may be adjusted not to be smaller /// than the decoding timestamp (dts) for the first fragment. @@ -100,7 +91,6 @@ class Fragmenter { bool StartsWithSAP() const; std::shared_ptr stream_info_; - bool use_decoding_timestamp_in_timeline_; TrackFragment* traf_; uint64_t seek_preroll_; bool fragment_initialized_; diff --git a/packager/media/formats/mp4/segmenter.cc b/packager/media/formats/mp4/segmenter.cc index 1d83e311c1..5115fadecf 100644 --- a/packager/media/formats/mp4/segmenter.cc +++ b/packager/media/formats/mp4/segmenter.cc @@ -71,10 +71,6 @@ Status Segmenter::Initialize( fragmenters_[i].reset(new Fragmenter(streams[i], &moof_->tracks[i])); } - if (options_.mp4_params.use_decoding_timestamp_in_timeline) { - for (uint32_t i = 0; i < streams.size(); ++i) - fragmenters_[i]->set_use_decoding_timestamp_in_timeline(true); - } // Only allow |EPT| to be adjusted for the first file. if (options_.output_file_index == 0) { for (uint32_t i = 0; i < streams.size(); ++i) diff --git a/packager/media/public/mp4_output_params.h b/packager/media/public/mp4_output_params.h index f9b83f1942..dd963cea18 100644 --- a/packager/media/public/mp4_output_params.h +++ b/packager/media/public/mp4_output_params.h @@ -24,11 +24,6 @@ struct Mp4OutputParams { static constexpr int kNoSidxBoxInSegment = -1; static constexpr int kSingleSidxPerSegment = 0; int num_subsegments_per_sidx = kSingleSidxPerSegment; - /// Set the flag use_decoding_timestamp_in_timeline, which if set to true, use - /// decoding timestamp instead of presentation timestamp in media timeline, - /// which is needed to workaround a Chromium bug that decoding timestamp is - /// used in buffered range, https://crbug.com/398130. - bool use_decoding_timestamp_in_timeline = false; }; } // namespace shaka