To make Gecko's media pipeline work for HLS, we need to provide these components which are derived from MediaDecoder/ MediaDataDemuxer / MediaTrackDemuxer / MediaResource.

Note: The core Java implementation for these patches will be provided in Bug 1350241 - [Fennec][HLS] Create a demuxer-like component based on ExoPlayer. These patches includes some debugging logs, once we pass the reviewed, I will removed all the debugging logs.

Comment on attachment 8864780 [details] Bug 1350246 - [Part0] Add a Util header for PR logging and define MOZ_ANDROID_HLS_SUPPORT macro. https://reviewboard.mozilla.org/r/136448/#review139914 ::: dom/media/hls/HLSUtils.h:14 (Diff revision 1) > + > +#include "mozilla/Logging.h" > +#include "MediaContainerType.h" > +// Logger > + > +inline no point making this inline

Comment on attachment 8864781 [details] Bug 1350246 - [Part1] Implement a HLSDecoder as a MediaDecoder. https://reviewboard.mozilla.org/r/136450/#review139916 ::: dom/media/hls/HLSDecoder.h:19 (Diff revision 2) > + > +class HLSDecoder final : public MediaDecoder > +{ > +public: > + // MediaDecoder interface. > + explicit HLSDecoder(MediaDecoderOwner* aOwner) : MediaDecoder(aOwner) {} space between { and }: { } ::: dom/media/hls/HLSDecoder.cpp:13 (Diff revision 2) > +#include "MediaContainerType.h" > +#include "MediaDecoderStateMachine.h" > +#include "MediaFormatReader.h" > +#include "MediaPrefs.h" > +#include "HLSDemuxer.h" > +#include "HLSUtils.h" headers to be alphabetically ordered, except HLSDecoder.h which is to be the first (as per coding style rules) ::: dom/media/hls/HLSDecoder.cpp:25 (Diff revision 2) > + MOZ_ASSERT(NS_IsMainThread()); > + HLS_DEBUG("HLSDecoder", ""); > + > + mReader = > + new MediaFormatReader(this, > + new HLSDemuxer(GetResource()), As discussed, I don't think we should be using a HLSResource wrapper/object it's unecessary and only complicate the matter ::: dom/media/hls/HLSDecoder.cpp:44 (Diff revision 2) > + > +bool > +HLSDecoder::IsEnabled() > +{ > + HLS_DEBUG_NON_MEMBER("HLSDecoder", "HLSEnabled = %d", MediaPrefs::HLSEnabled()); > + return MediaPrefs::HLSEnabled(); should also check we're on supported version of android. ::: dom/media/hls/HLSDecoder.cpp:52 (Diff revision 2) > +bool > +HLSDecoder::IsSupportedType(const MediaContainerType& aContainerType) > +{ > + HLS_DEBUG_NON_MEMBER("HLSDecoder", "HLSDecoder::aContainerType = %s", aContainerType.OriginalString().Data()); > + HLS_DEBUG_NON_MEMBER("HLSDecoder", "HLSDecoder::IsSupportedType = %d", IsHttpLiveStreamingType(aContainerType)); > + return IsEnabled() trailing &&

Comment on attachment 8864782 [details] Bug 1350246 - [Part2] Implement a HLSResource as a MediaResource. https://reviewboard.mozilla.org/r/136452/#review139918 We should be able to do without this class... it adds no value and the default from the base class are sufficient for our use. ::: dom/media/hls/HLSResource.h:93 (Diff revision 2) > + const MediaContainerType& GetContentType() const override { return mContainerType; } > + > + bool IsLiveStream() override > + { > + MonitorAutoLock mon(mMonitor); > + // TODO: return false and need to know what is that API used for? figure it out :)

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review139920 Please clarify your threading model by having the proper MOZ_ASSERT at the beginning of each function. In particular with the resource callbacks. ::: dom/media/hls/HLSDemuxer.h:68 (Diff revision 2) > + nsTArray<RefPtr<HLSTrackDemuxer>> mDemuxers; > + > + MozPromiseHolder<InitPromise> mInitPromise; > + > + // Monitor to protect members below across multiple threads. > + mutable Monitor mMonitor; please use a Mutex instead. ::: dom/media/hls/HLSDemuxer.cpp:137 (Diff revision 2) > +{ > + HLS_DEBUG("HLSDemuxer", "NotifyDataArrived()"); > + if (mInitPromise.IsEmpty()) { > + return; > + } > + mInitPromise.ResolveIfExists(NS_OK, __func__); why that? How can getting NotifyDataArrived be sufficient to determine if the metadata have been retrieved or not? if that's the case, it should have a comment explaining the why this is also not thread safe.. mInitPromise is accessed from both the demuxer's taskqueue and the reader's taskqueue ::: dom/media/hls/HLSDemuxer.cpp:161 (Diff revision 2) > +HLSDemuxer::GetNumberTracks(TrackType aType) const > +{ > + HLS_DEBUG("HLSDemuxer", "GetNumberTracks(%d)", aType); > + switch (aType) { > + case TrackType::kAudioTrack: > + return mHlsDemuxerWrapper->GetNumberOfTracks(1); use an enum ::: dom/media/hls/HLSDemuxer.cpp:181 (Diff revision 2) > +} > + > +bool > +HLSDemuxer::IsSeekable() const > +{ > + return true; what about live streams? They are typically not seekable ::: dom/media/hls/HLSDemuxer.cpp:189 (Diff revision 2) > +UniquePtr<EncryptionInfo> > +HLSDemuxer::GetCrypto() > +{ > + MonitorAutoLock mon(mMonitor); > + auto crypto = MakeUnique<EncryptionInfo>(); > + *crypto = mInfo.mCrypto; i would always return nullptr for now. the crypto returned has nothing to do with the one expected by EME and the MediaFormatReader ::: dom/media/hls/HLSDemuxer.cpp:224 (Diff revision 2) > + MOZ_ASSERT(mHlsDemuxerWrapper); > + HLS_DEBUG("HLSDemuxer", "UpdateAudioInfo (%d)", index); > + jni::Object::LocalRef infoObj = mHlsDemuxerWrapper->GetAudioInfo(index); > + if (infoObj) { > + java::GeckoAudioInfo::LocalRef audioInfo(mozilla::Move(infoObj)); > + mInfo.mAudio.mRate = audioInfo->Rate(); your header comment indicates that mMonitor protects mInfo, yet you don't lock. Also, having assert indicating where each methods are running would help ::: dom/media/hls/HLSDemuxer.cpp:230 (Diff revision 2) > + mInfo.mAudio.mChannels = audioInfo->Channels(); > + mInfo.mAudio.mProfile = audioInfo->Profile(); > + mInfo.mAudio.mBitDepth = audioInfo->BitDepth(); > + mInfo.mAudio.mMimeType = NS_ConvertUTF16toUTF8(audioInfo->MimeType()->ToString()); > + mInfo.mAudio.mDuration = TimeUnit::FromMicroseconds(audioInfo->Duration()); > + auto&& csd = audioInfo->CodecSpecificData()->GetElements(); that can't be a &&, GetElements() doesn't return moved data/rvalue ::: dom/media/hls/HLSDemuxer.cpp:244 (Diff revision 2) > +{ > + MOZ_ASSERT(mHlsDemuxerWrapper); > + jni::Object::LocalRef infoObj = mHlsDemuxerWrapper->GetVideoInfo(index); > + if (infoObj) { > + java::GeckoVideoInfo::LocalRef videoInfo(mozilla::Move(infoObj)); > + mInfo.mVideo.mStereoMode = getStereoMode(videoInfo->StereoMode()); same deal as UpdateAudioInfo... OnInitialized is marked as being called from the MainThread. So now you have mInfo being accessed from 3 different threads with no use of monitor/mutex ::: dom/media/hls/HLSDemuxer.cpp:247 (Diff revision 2) > + if (infoObj) { > + java::GeckoVideoInfo::LocalRef videoInfo(mozilla::Move(infoObj)); > + mInfo.mVideo.mStereoMode = getStereoMode(videoInfo->StereoMode()); > + mInfo.mVideo.mRotation = getVideoInfoRotation(videoInfo->Rotation()); > + //TODO: Check how to get this information since DecoderInputBuffer did not provide this info. > + mInfo.mVideo.mImage.width = videoInfo->DisplayX(); can get this info by decoding the SPS NAL ::: dom/media/hls/HLSDemuxer.cpp:254 (Diff revision 2) > + mInfo.mVideo.mDisplay.width = videoInfo->DisplayX(); > + mInfo.mVideo.mDisplay.height = videoInfo->DisplayY(); > + mInfo.mVideo.mMimeType = NS_ConvertUTF16toUTF8(videoInfo->MimeType()->ToString()); > + mInfo.mVideo.mDuration = TimeUnit::FromMicroseconds(videoInfo->Duration()); > + // We don't need H264Converter to be involved since MediaCodec accept the sample format we provided. > + mInfo.mVideo.mNeedConversion = false; where is this member defined? missing a patch somewhere? ::: dom/media/hls/HLSDemuxer.cpp:329 (Diff revision 2) > + if (!aNumSamples) { > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_DEMUXER_ERR, > + __func__); > + } > + RefPtr<SamplesHolder> samples = new SamplesHolder; > + //TRACK_AUDIO = 1; using an explicit enum would prevent such confusion ::: dom/media/hls/HLSDemuxer.cpp:341 (Diff revision 2) > + > + if (sampleObjectArray.IsEmpty()) { > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_WAITING_FOR_DATA, __func__); > + } > + > + for (auto&& demuxedSample : sampleObjectArray) { those are not rvalues. auto& ::: dom/media/hls/HLSDemuxer.cpp:355 (Diff revision 2) > + mrd->mTime = TimeUnit::FromMicroseconds(presentationTimeUs); > + mrd->mTimecode = TimeUnit::FromMicroseconds(presentationTimeUs); > + > + if (sample->IsEOS()) { > + // TODO: consider using an elegant way to handle this case. > + HLS_DEBUG("HLSTrackDemuxer", "Met BUFFER_FLAG_END_OF_STREAM."); what's not elegant about it ? ::: dom/media/hls/HLSDemuxer.cpp:361 (Diff revision 2) > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, __func__); > + } > + > + mrd->mKeyframe = sample->IsKeyFrame(); > + // TODO : Fix this when we know how to calculate from ExoPlayer. > + mrd->mDuration = (mType == TrackInfo::kVideoTrack) ? TimeUnit::FromMicroseconds(sample->Duration()) : TimeUnit::Zero(); 80 columns formatting, and please figure that out first. ::: dom/media/hls/HLSDemuxer.cpp:381 (Diff revision 2) > + mrd->mTrackInfo = new TrackInfoSharedPtr(mParent->mInfo.mVideo, ++sStreamSourceID); > + } > + } > + > + // Write payload into MediaRawData > + UniquePtr<MediaRawDataWriter> writer(mrd->CreateWriter()); you could use the MediaRawSample(size) constructor instead. ::: dom/media/hls/HLSDemuxer.cpp:383 (Diff revision 2) > + } > + > + // Write payload into MediaRawData > + UniquePtr<MediaRawDataWriter> writer(mrd->CreateWriter()); > + if (!writer->SetSize(size)) { > + HLS_DEBUG("HLSTrackDemuxer", "Exit failed to allocated media buffer"); failed to allocate ::: dom/media/hls/HLSDemuxer.cpp:444 (Diff revision 2) > + sample->Dispose(); > + } > + > + if (mType == TrackInfo::kVideoTrack) { > + // Only need to find NextKeyFrame for Video > + UpdateNextKeyFrameTime(); you only need to get the next keyframe, when the last demuxed frame has a time > keyframetime. no need to recalculate it all the time ::: dom/media/hls/HLSDemuxer.cpp:461 (Diff revision 2) > +void > +HLSTrackDemuxer::UpdateNextKeyFrameTime() > +{ > + MOZ_ASSERT(mParent, "Called after BreackCycle()"); > + int64_t nextKeyFrameTime = mParent->GetNextKeyFrameTime(); > + if (nextKeyFrameTime != mNextKeyframeTime.valueOr(media::TimeUnit::Zero()).ToMicroseconds()) { if you don't know the keyframe time, it should be MAX_INT (or TimeUnit::FromInfinity) ::: dom/media/hls/HLSDemuxer.cpp:463 (Diff revision 2) > +{ > + MOZ_ASSERT(mParent, "Called after BreackCycle()"); > + int64_t nextKeyFrameTime = mParent->GetNextKeyFrameTime(); > + if (nextKeyFrameTime != mNextKeyframeTime.valueOr(media::TimeUnit::Zero()).ToMicroseconds()) { > + HLS_DEBUG("HLSTrackDemuxer", "Update nextKeyFrameTime to %lld", nextKeyFrameTime); > + mNextKeyframeTime.reset(); mNextKeyframe = Some(...) ::: dom/media/hls/HLSDemuxer.cpp:485 (Diff revision 2) > + > +RefPtr<HLSTrackDemuxer::SkipAccessPointPromise> > +HLSTrackDemuxer::SkipToNextRandomAccessPoint(const media::TimeUnit& aTimeThreshold) > +{ > + uint32_t parsed = 0; > + SkipFailureHolder holder(NS_ERROR_DOM_MEDIA_WAITING_FOR_DATA, parsed); you don't want that... this will cause a permanent stall should any frames by dropped ::: dom/media/hls/HLSDemuxer.cpp:494 (Diff revision 2) > +media::TimeIntervals > +HLSTrackDemuxer::GetBuffered() > +{ > + MOZ_ASSERT(mParent, "Called after BreackCycle()"); > + int64_t bufferedTime = mParent->mHlsDemuxerWrapper->GetBuffered(); //us > + return TimeIntervals(TimeInterval(TimeUnit(), TimeUnit::FromMicroseconds(bufferedTime))); surely, the exoplayer discard data as it plays... can't we know when the buffered range start?

Comment on attachment 8864784 [details] Bug 1350246 - [Part4] Add a feature preference for HLS, default on for Fennec. https://reviewboard.mozilla.org/r/136456/#review139922 ::: dom/media/MediaPrefs.h:176 (Diff revision 2) > DECL_MEDIA_PREF("media.ogg.flac.enabled", FlacInOgg, bool, false); > DECL_MEDIA_PREF("media.flac.enabled", FlacEnabled, bool, true); > > + // Hls > + // TODO: Default to true only for testing it on Desktop. > + DECL_MEDIA_PREF("media.hls.enabled", HLSEnabled, bool, true); default should be false. and preference only visible on android

Comment on attachment 8864785 [details] Bug 1350246 - [Part5] Move IsHttpLiveStreamingType from DecoderTraits.cpp to its .h. and modify the callers related to this change. https://reviewboard.mozilla.org/r/136458/#review139924 this belongs to HLSDecoder ::: dom/media/DecoderTraits.cpp:179 (Diff revision 2) > CanHandleMediaType(const MediaContainerType& aType, > DecoderDoctorDiagnostics* aDiagnostics) > { > MOZ_ASSERT(NS_IsMainThread()); > > +#ifndef MOZ_ANDROID_OMX why is this code be made conditional on MOZ_ANDROID_OMX? this seems out of scope ::: dom/media/DecoderTraits.cpp:339 (Diff revision 2) > decoder = new DirectShowDecoder(aOwner); > return decoder.forget(); > } > #endif > > +#ifndef MOZ_ANDROID_OMX same ::: dom/media/VideoUtils.h:552 (Diff revision 2) > } > } > return false; > } > > +bool IsHttpLiveStreamingType(const mozilla::MediaContainerType& aType); should move this into the HLSDecoder

Comment on attachment 8864786 [details] Bug 1350246 - [Part7] Create HLSResource if the mimetype is HLS. https://reviewboard.mozilla.org/r/136460/#review139926 as commented earlier, I think we could do without... ::: commit-message-8c022:1 (Diff revision 2) > +Bug 1350246 - [Part6] Create HLSResource depends on the mimetype if it is HLS. not sure I understand the comment...

Comment on attachment 8864787 [details] Bug 1350246 - [Part8] Add the native code implementation into source tree. https://reviewboard.mozilla.org/r/136462/#review139928 ::: commit-message-15fe1:1 (Diff revision 2) > +Bug 1350246 - [Part7] Define MOZ_ANDROID_HLS_SUPPORT macro and add the native code into source tree. the MOZ_ANDROID_HLS_SUPPORT is used in earlier parts.. So this patch can't be made to be last. ::: dom/media/hls/moz.build:14 (Diff revision 2) > + 'HLSDemuxer.h', > + 'HLSResource.h', > + 'HLSUtils.h', > +] > + > +EXPORTS.mozilla.dom += [ not needed

Can you please close/respond to the issues that you've fixed, dropped or whatever? it makes it very hard to review when there are over 20 issues still appearing as pending... thank you.

Comment on attachment 8864785 [details] Bug 1350246 - [Part5] Move IsHttpLiveStreamingType from DecoderTraits.cpp to its .h. and modify the callers related to this change. https://reviewboard.mozilla.org/r/136458/#review147446 ::: dom/media/DecoderTraits.cpp (Diff revisions 2 - 3) > if (MediaDecoder::IsAndroidMediaPluginEnabled()) { > EnsureAndroidMediaPluginHost()->FindDecoder(aType, &supportedCodecs); > } > #endif > -#ifdef MOZ_ANDROID_HLS_SUPPORT > - if (HLSDecoder::IsSupportedType(mimeType)) { This change doesn't match what is described in the commit. This isn't about moving the definition of a function, but changing the behaviour of the existing code. Please either clarify the scope of this change, create/split a new patch if required.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review147450 ::: dom/media/hls/HLSDemuxer.h:72 (Diff revisions 2 - 3) > > - // Monitor to protect members below across multiple threads. > - mutable Monitor mMonitor; > + // Mutex to protect members below across multiple threads. > + mutable Mutex mMutex; > MediaInfo mInfo; > > java::GeckoHlsDemuxerWrapper::HlsDemuxerCallbacks::GlobalRef mJavaCallbacks; you use HLS in most classes, but use Hls in member names. Please use HLS not Hls ::: dom/media/hls/HLSDemuxer.cpp:85 (Diff revisions 2 - 3) > - mDemuxer->OnInitialized(aHasAudio, aHasVideo); > + mDemuxer->OnInitialized(aHasAudio, aHasVideo); > + })); > - } > + } > > - void OnDemuxerError(int aErrorCode) {} > + // TODO: Handle the unexpected error signal from the java implementation. > + void OnError(int aErrorCode) { } please open a bug for this task and use bug number here. ::: dom/media/hls/HLSDemuxer.cpp:99 (Diff revisions 2 - 3) > /* aSupportsTailDispatch = */ false)) > - , mMonitor("HLSDemuxer") > + , mMutex("HLSDemuxer") > { > MOZ_ASSERT(NS_IsMainThread()); > MOZ_ASSERT(aResource); > - HlsDemuxerCallbacksSupport::Init(); > + HlsDemuxerCallbacksSupport::Init(); bad indent ::: dom/media/hls/HLSDemuxer.cpp:122 (Diff revisions 2 - 3) > UpdateAudioInfo(0); > } > if (aHasVideo) { > UpdateVideoInfo(0); > } > + if (mInitPromise.IsEmpty()) { don't need this if you use ResolveIfExists. Additionally, the Init Promise can never be empty here, as the only place mInitPromise is used is in the destructor. ::: dom/media/hls/HLSDemuxer.cpp:189 (Diff revisions 2 - 3) > } > > UniquePtr<EncryptionInfo> > HLSDemuxer::GetCrypto() > { > - MonitorAutoLock mon(mMonitor); > + // TODO: Currently, our HLS implementation hasn't supported encrypted content. doesn't support ::: dom/media/hls/HLSDemuxer.cpp:368 (Diff revisions 2 - 3) > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_DEMUXER_ERR, __func__); > + } > + samples->mSamples.AppendElement(mrd); > + } > + if (mType == TrackInfo::kVideoTrack && > + (mNextKeyframeTime.isNothing() be consistent with the use of && or || have them at the end of the line per gecko coding style. ::: dom/media/hls/HLSDemuxer.cpp:389 (Diff revisions 2 - 3) > - int64_t presentationTimeUs = 0; > + int64_t presentationTimeUs = 0; > - bool ok = NS_SUCCEEDED(info->PresentationTimeUs(&presentationTimeUs)); > + bool ok = NS_SUCCEEDED(info->PresentationTimeUs(&presentationTimeUs)); > - mrd->mTime = TimeUnit::FromMicroseconds(presentationTimeUs); > + mrd->mTime = TimeUnit::FromMicroseconds(presentationTimeUs); > - mrd->mTimecode = TimeUnit::FromMicroseconds(presentationTimeUs); > + mrd->mTimecode = TimeUnit::FromMicroseconds(presentationTimeUs); > + mrd->mKeyframe = aSample->IsKeyFrame(); > + mrd->mDuration = (mType == TrackInfo::kVideoTrack) ? ? at the beginning of the next line, per coding style rules. ::: dom/media/hls/HLSDemuxer.cpp:478 (Diff revisions 2 - 3) > void > HLSTrackDemuxer::UpdateNextKeyFrameTime() > { > MOZ_ASSERT(mParent, "Called after BreackCycle()"); > int64_t nextKeyFrameTime = mParent->GetNextKeyFrameTime(); > - if (nextKeyFrameTime != mNextKeyframeTime.valueOr(media::TimeUnit::Zero()).ToMicroseconds()) { > + if (nextKeyFrameTime != mNextKeyframeTime.valueOr(media::TimeUnit::FromInfinity()).ToMicroseconds()) { it makes no sense to convert FromInfinity to microseconds. Instead, change nextKeyFrameTime to be TimeUnit::Infinity when it needs to be and compare with that. ::: dom/media/hls/HLSDemuxer.cpp:490 (Diff revisions 2 - 3) > HLSTrackDemuxer::GetNextRandomAccessPoint(media::TimeUnit* aTime) > { > if (mNextKeyframeTime.isNothing()) { > // There's no next key frame. > *aTime = > media::TimeUnit::FromMicroseconds(std::numeric_limits<int64_t>::max()); Use TimeUnit::FromInfinity() ::: dom/media/hls/HLSDemuxer.cpp:514 (Diff revisions 2 - 3) > + const TimeUnit& aTimeThreshold) > { > + mQueuedSample = nullptr; > uint32_t parsed = 0; > + bool found = false; > + while (!found) { use do {} while() so you avoid the unnecessary first test. additionally, in your block, you always return and/or break when found is true. ::: dom/media/hls/HLSDemuxer.cpp:516 (Diff revisions 2 - 3) > + mQueuedSample = nullptr; > uint32_t parsed = 0; > + bool found = false; > + while (!found) { > + mozilla::jni::ObjectArray::LocalRef demuxedSamples = > + (mType == TrackInfo::kAudioTrack) ? ? at the beginning of the next line ::: dom/media/hls/HLSDemuxer.cpp:549 (Diff revisions 2 - 3) > media::TimeIntervals > HLSTrackDemuxer::GetBuffered() > { > MOZ_ASSERT(mParent, "Called after BreackCycle()"); > - int64_t bufferedTime = mParent->mHlsDemuxerWrapper->GetBuffered(); //us > + //int64_t bufferedTime = mParent->mHlsDemuxerWrapper->GetBuffered(); //us > + int64_t bufferedTime = 0; is this intended?

(In reply to Jean-Yves Avenard [:jya] from comment #33) > Can you please close/respond to the issues that you've fixed, dropped or > whatever? > > it makes it very hard to review when there are over 20 issues still > appearing as pending... > > thank you. Apologies for make you feel uncomfortable when reviewing the code. I will do this and refine the patches you mentioned above. Thanks for your reminding.

Comment on attachment 8864785 [details] Bug 1350246 - [Part5] Move IsHttpLiveStreamingType from DecoderTraits.cpp to its .h. and modify the callers related to this change. https://reviewboard.mozilla.org/r/136458/#review147446 > This change doesn't match what is described in the commit. > > This isn't about moving the definition of a function, but changing the behaviour of the existing code. > > > Please either clarify the scope of this change, create/split a new patch if required. I have seperated it to part6 and amended the commit message, Thanks

Comment on attachment 8864785 [details] Bug 1350246 - [Part5] Move IsHttpLiveStreamingType from DecoderTraits.cpp to its .h. and modify the callers related to this change. It's weird that the status is not sync in review board.

you still have 32 opened issues on part3. ni? me when those patches are ready to be reviewed.

Comment on attachment 8872953 [details] Bug 1350246 - [Part6] Use DecoderTraits::IsHttpLiveStreamingType to implement HLSDecoder::IsSupportedType and use HLSDecoder::IsSupportedType to implement the following APIs for mimetype of HLS. https://reviewboard.mozilla.org/r/144488/#review148304 ::: dom/media/DecoderTraits.cpp:335 (Diff revision 1) > decoder = new DirectShowDecoder(aOwner); > return decoder.forget(); > } > #endif > > +#ifdef MOZ_ANDROID_HLS_SUPPORT you made CanHandleMediaType test for HLS first. Please make it consistent. either change it so it's tested last or move the creation of the HLSDecoder to be first.

Comment on attachment 8864785 [details] Bug 1350246 - [Part5] Move IsHttpLiveStreamingType from DecoderTraits.cpp to its .h. and modify the callers related to this change. https://reviewboard.mozilla.org/r/136458/#review148306

Comment on attachment 8872953 [details] Bug 1350246 - [Part6] Use DecoderTraits::IsHttpLiveStreamingType to implement HLSDecoder::IsSupportedType and use HLSDecoder::IsSupportedType to implement the following APIs for mimetype of HLS. https://reviewboard.mozilla.org/r/144488/#review148304 > you made CanHandleMediaType test for HLS first. Please make it consistent. either change it so it's tested last or move the creation of the HLSDecoder to be first. Thank you, I will move the creation of the HLSDecoder to be first. And I will r? and cleanup the issue list when I finish refining that patch. Many thanks.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review139920 > what about live streams? > > They are typically not seekable Implemented by querying from Exoplayer. > your header comment indicates that mMonitor protects mInfo, yet you don't lock. > > Also, having assert indicating where each methods are running would help Thanks for reminding, I've used mutex to protect the mInfo > you could use the MediaRawSample(size) constructor instead. This constructor here http://searchfox.org/mozilla-central/source/dom/media/MediaData.cpp#461 needs a raw buffer in advance and I would like to use the current approach to handle if the buffer allocation failed instead of using constructor. > surely, the exoplayer discard data as it plays... can't we know when the buffered range start? currently, exoplayer cannot offer us the start of the buffered range.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review147450 > you use HLS in most classes, but use Hls in member names. > > Please use HLS not Hls The modification involved the java part implementation, so I decided to do this change in https://bugzilla.mozilla.org/show_bug.cgi?id=1368907#c0 > is this intended? no, my bad, I forget to remove the testing code. sorry.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review148786 LGTM, but please document your threading model with the appropriate use of assertions. I believe you have threading/race issue between the callback method and how it returns value to the demuxer. We had similar issues in the RemoteDataDecoder and I don't see anything in place to prevent the callback from using the demuxer after it's been destructed as it uses a weak pointer. ::: dom/media/hls/HLSDemuxer.h:26 (Diff revision 5) > + > +class AbstractThread; > +class MediaResult; > +class HLSTrackDemuxer; > + > +class HLSDemuxer : public MediaDataDemuxer can make the class final ::: dom/media/hls/HLSDemuxer.h:52 (Diff revision 5) > + > + AutoTaskQueue* GetTaskQueue() { return mTaskQueue; } > + void OnInitialized(bool aHasAudio, bool aHasVideo); > + > +protected: > + int64_t GetNextKeyFrameTime(); please use TimeUnit as much as possible ::: dom/media/hls/HLSDemuxer.h:63 (Diff revision 5) > + ~HLSDemuxer(); > + RefPtr<MediaResource> mResource; > + friend class HLSTrackDemuxer; > + TrackInfo* GetTrackInfo(TrackInfo::TrackType); > + > + RefPtr<AutoTaskQueue> mTaskQueue; please make this a const RefPtr ::: dom/media/hls/HLSDemuxer.cpp:13 (Diff revision 5) > +#include <limits> > +#include <stdint.h> > + > +#include "GeneratedJNINatives.h" > +#include "GeneratedJNIWrappers.h" > +#include "HLSDemuxer.h" HLSDemuxer.h should be the first in the list of include as per coding style ::: dom/media/hls/HLSDemuxer.cpp:18 (Diff revision 5) > +#include "HLSDemuxer.h" > +#include "HLSResource.h" > +#include "HLSUtils.h" > +#include "MediaCodec.h" > +#include "nsPrintfCString.h" > +#include "mozilla/Unused.h" nsPrint > mozilla, please order by alphabetical order ::: dom/media/hls/HLSDemuxer.cpp:32 (Diff revision 5) > +using media::TimeUnit; > +using media::TimeIntervals; > +using media::TimeInterval; > + > +static > +VideoInfo::Rotation getVideoInfoRotation(int aRotation) { brace at the start of the newline, her and everywhere it's incorrectly placed. ::: dom/media/hls/HLSDemuxer.cpp:78 (Diff revision 5) > + } > + > + void OnInitialized(bool aHasAudio, bool aHasVideo) > + { > + MOZ_ASSERT(mDemuxer); > + mDemuxer->GetTaskQueue()->Dispatch(NS_NewRunnableFunction( there's seems to be potential for UAF here if you dispatch a task like that. How do you guarantee the lifetime of HlsDemuxerCallbacksSupport and of HLSDemuxer? I assumer that they run on different threads (I must make that assumptions as nothing in the code document the threading model) Having a detach method that would ensure that mDemuxer isn't used once the callback has been detached ::: dom/media/hls/HLSDemuxer.cpp:108 (Diff revision 5) > + > + HlsDemuxerCallbacksSupport::AttachNative(mJavaCallbacks, > + mozilla::MakeUnique<HlsDemuxerCallbacksSupport>(this)); > + > + auto resourceWrapper = static_cast<HLSResource*>(aResource)->GetResourceWrapper(); > + mHlsDemuxerWrapper = GeckoHlsDemuxerWrapper::Create(resourceWrapper->GetPlayer(), mJavaCallbacks); is GeckoHlsDemuxerWrapper thread-safe? it's called over different threads here (the HLSDemuxer task queue, and the MediaDecoderReader taskqueue) ::: dom/media/hls/HLSDemuxer.cpp:225 (Diff revision 5) > + MOZ_ASSERT(mHlsDemuxerWrapper); > + HLS_DEBUG("HLSDemuxer", "UpdateAudioInfo (%d)", index); > + MutexAutoLock lock(mMutex); > + jni::Object::LocalRef infoObj = mHlsDemuxerWrapper->GetAudioInfo(index); > + if (infoObj) { > + java::GeckoAudioInfo::LocalRef audioInfo(mozilla::Move(infoObj)); you're already in the mozilla namespace, you don't need to specify it ::: dom/media/hls/HLSDemuxer.cpp:272 (Diff revision 5) > + > +HLSDemuxer::~HLSDemuxer() > +{ > + HLS_DEBUG("HLSDemuxer", "~HLSDemuxer()"); > + if (mJavaCallbacks) { > + HlsDemuxerCallbacksSupport::DisposeNative(mJavaCallbacks); two spaces indent ::: dom/media/hls/HLSDemuxer.cpp:360 (Diff revision 5) > + > + for (auto&& demuxedSample : sampleObjectArray) { > + java::GeckoHlsSample::LocalRef sample(mozilla::Move(demuxedSample)); > + if (sample->IsEOS()) { > + HLS_DEBUG("HLSTrackDemuxer", "Met BUFFER_FLAG_END_OF_STREAM."); > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, __func__); this assumes that sampleObjectArray will only ever contains a single EOS sample. Is that true? otherwise, you could drop preceding sample here. if sampleObjectArray can only ever have one sample in it, while loop over the array? ::: dom/media/hls/HLSDemuxer.cpp:364 (Diff revision 5) > + HLS_DEBUG("HLSTrackDemuxer", "Met BUFFER_FLAG_END_OF_STREAM."); > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, __func__); > + } > + RefPtr<MediaRawData> mrd = ConvertToMediaRawData(sample); > + if (!mrd) { > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_DEMUXER_ERR, __func__); I'm guessing here we have an OOM error, please reject with OOM error code ::: dom/media/hls/HLSDemuxer.cpp:379 (Diff revision 5) > + > + return SamplesPromise::CreateAndResolve(samples, __func__); > +} > + > +RefPtr<MediaRawData> > +HLSTrackDemuxer::ConvertToMediaRawData(java::GeckoHlsSample::LocalRef aSample) { braces at the beginning of the next line... this is not consistent too ::: dom/media/hls/HLSDemuxer.cpp:427 (Diff revision 5) > + java::sdk::CryptoInfo::LocalRef cryptoInfo = aSample->CryptoInfo(); > + if (cryptoInfo) { > + HLS_DEBUG("HLSTrackDemuxer", "Has Crypto Info"); > + writer->mCrypto.mValid = true; > + int32_t mode = 0; > + ok &= NS_SUCCEEDED(cryptoInfo->Mode(&mode)); can't you stop early of you have an error ? this keeps going even with errors. You should put that whole code in another function and return early. ::: dom/media/hls/HLSDemuxer.cpp:521 (Diff revision 5) > + bool found = false; > + MediaResult result = NS_ERROR_DOM_MEDIA_END_OF_STREAM; > + do { > + mozilla::jni::ObjectArray::LocalRef demuxedSamples = > + (mType == TrackInfo::kAudioTrack) > + ? mParent->mHlsDemuxerWrapper->GetSamples(TrackInfo::kAudioTrack, 1) this whole bit can be replaced with mParent->mHlsDemuxerWrapper->GetSamples(mType, 1) ::: dom/media/hls/HLSDemuxer.cpp:569 (Diff revision 5) > +HLSTrackDemuxer::BreakCycles() > +{ > + RefPtr<HLSTrackDemuxer> self = this; > + nsCOMPtr<nsIRunnable> task = > + NS_NewRunnableFunction([self]() { > + self->mParent = nullptr; you clear mParent on the demuxer's taskqueue but typically access mParent on the reader's taskqueue.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review148786 Thanks for you deep review! Please see my comment below, > there's seems to be potential for UAF here if you dispatch a task like that. > > How do you guarantee the lifetime of HlsDemuxerCallbacksSupport and of HLSDemuxer? > I assumer that they run on different threads (I must make that assumptions as nothing in the code document the threading model) > > Having a detach method that would ensure that mDemuxer isn't used once the callback has been detached I've added some comments that we will unregister this callback instance when HlsDemuxerCallbacksSupport::DisposeNative(mJavaCallbacks) had been called. We will ensure the java side will stop invoking this callback to avoid the UAF. > is GeckoHlsDemuxerWrapper thread-safe? > > it's called over different threads here (the HLSDemuxer task queue, and the MediaDecoderReader taskqueue) The methods inside GeckoHlsDemuxerWrapper are not all thread-safe. We inspect all the API usage and once we found the racy occurred, we will use the 'synchronized' java keyword to make it thread-safe. Currently, we haven't met the wrong result caused by race condition. As our plan, we want to land the code into nightly to make QA test our code. In parallel, we start working on "Bug 1357984 - Add test case for HLS native supprts" to find the racy or code defects. Thank you. > this assumes that sampleObjectArray will only ever contains a single EOS sample. > > Is that true? > > otherwise, you could drop preceding sample here. > > if sampleObjectArray can only ever have one sample in it, while loop over the array? Yes, it's true. I've modified this part. when I got a EOS sample(in requesting multiple sample case), I will cache the sample and resolve the promise with the preceding samples. > can't you stop early of you have an error ? > > this keeps going even with errors. > > You should put that whole code in another function and return early. I've moved those code into a function and check the return value per API call and logged the error msg. Thanks. > you clear mParent on the demuxer's taskqueue but typically access mParent on the reader's taskqueue. Thanks for reminding. Frankly, I just referenced this code from http://searchfox.org/mozilla-central/rev/d441cb24482c2e5448accaf07379445059937080/dom/media/mediasource/MediaSourceDemuxer.cpp#388 Does it have the same problem?

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review148786 > Thanks for reminding. > > Frankly, I just referenced this code from http://searchfox.org/mozilla-central/rev/d441cb24482c2e5448accaf07379445059937080/dom/media/mediasource/MediaSourceDemuxer.cpp#388 > > Does it have the same problem? it does, but you have everywhere in the code MOZ_ASSERT(mParent, "Called after BreackCycle()"); everytime mParent is accessed on the reader's taskqueue.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review150360 We're almost there. But I worry about the chance that we will have a UAF if MFR::Shutdown is called right during demuxer's initialisation operation (and it does happen, we've had lots of issues there in the past under that exact scenario) ::: dom/media/hls/HLSDemuxer.cpp:67 (Diff revisions 5 - 6) > +// HlsDemuxerCallbacksSupport is a native implemented callback class for > +// HlsDemuxerCallbacks in GeckoHlsDemuxerWrapper.java. > +// The callback functions will be invoked by JAVA-side thread. > +// Should dispatch the task to the demuxer's task queue. > +// We ensure the callback will never be invoked after > +// HlsDemuxerCallbacksSupport::DisposeNative has been called in ~HLSDemuxer. sorry, but as far as I can tell, this is still racy... Imagine the following case: The java callback calls OnInitialised, which queue a task on the taskqueue. Right at the same time this is occurring, the MediaFormatReader::Shutdown is called, the demuxer destructor is called , mDemuxer is destroyed. The task queue by OnInitialised now runs: mDemuxer is no longer a valid pointer: UAF. Yes, the callback won't receive a new call, but pending calls aren't being taken care off. This is one of the major issue with callbacks like this that are running on different threads and you don't have a strong reference ensure the pointer is kept alive. ::: dom/media/hls/HLSDemuxer.cpp:382 (Diff revisions 5 - 6) > for (auto&& demuxedSample : sampleObjectArray) { > - java::GeckoHlsSample::LocalRef sample(mozilla::Move(demuxedSample)); > + java::GeckoHlsSample::LocalRef sample(Move(demuxedSample)); > if (sample->IsEOS()) { > HLS_DEBUG("HLSTrackDemuxer", "Met BUFFER_FLAG_END_OF_STREAM."); > - return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, __func__); > + if (samples->mSamples.IsEmpty()) { > + return SamplesPromise::CreateAndReject(NS_ERROR_DOM_MEDIA_END_OF_STREAM, I don't really understand the logic here. You're rejecting the promise with EOS, but set mQueuedSample with mEOS to true so that the next call to GetSample will once again return EOS? What's the point? do you need mQueuedSample here? ::: dom/media/hls/HLSDemuxer.cpp:415 (Diff revisions 5 - 6) > + // Extract Crypto information > + CryptoSample crypto; > + bool ok = false; > + char const* msg = ""; > + do { > + if (!aCryptoInfo) { please put this outside of the loop ::: dom/media/hls/HLSDemuxer.cpp:421 (Diff revisions 5 - 6) > + return CryptoSample{}; > + } > + HLS_DEBUG("HLSTrackDemuxer", "Sample has Crypto Info"); > + crypto.mValid = true; > + int32_t mode = 0; > + ok = NS_SUCCEEDED(aCryptoInfo->Mode(&mode)); I would much prefer to have things like: if (NS_FAILED(blah)) { msg = "..."; break; } as having ok helps nothing here ::: dom/media/hls/HLSDemuxer.cpp:540 (Diff revisions 5 - 6) > > void > HLSTrackDemuxer::Reset() > { > MOZ_ASSERT(mParent, "Called after BreackCycle()"); > + mQueuedSample = nullptr; by definition, with Reset() it should be equivalent to doing Seek(0). but I guess it doesn't matter, as Seek() is always called after a Reset() ::: dom/media/hls/HLSDemuxer.cpp:548 (Diff revisions 5 - 6) > void > HLSTrackDemuxer::UpdateNextKeyFrameTime() > { > MOZ_ASSERT(mParent, "Called after BreackCycle()"); > - media::TimeUnit nextKeyFrameTime = > - media::TimeUnit::FromMicroseconds(mParent->GetNextKeyFrameTime()); > + TimeUnit nextKeyFrameTime = mParent->GetNextKeyFrameTime(); > + if (nextKeyFrameTime != mNextKeyframeTime.valueOr(TimeUnit::FromInfinity())) { refOr would be better, avoid an unnecessary copy. ::: dom/media/hls/HLSDemuxer.cpp:559 (Diff revisions 5 - 6) > nsresult > -HLSTrackDemuxer::GetNextRandomAccessPoint(media::TimeUnit* aTime) > +HLSTrackDemuxer::GetNextRandomAccessPoint(TimeUnit* aTime) > { > if (mNextKeyframeTime.isNothing()) { > // There's no next key frame. > *aTime = that should fit within 80 columns, no need for the new line ::: dom/media/hls/HLSDemuxer.cpp:633 (Diff revisions 5 - 6) > } > > void > HLSTrackDemuxer::BreakCycles() > { > - RefPtr<HLSTrackDemuxer> self = this; > + mParent = nullptr; no, you do want to clear mParent on the taskqueue, the previous code was better.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review150360 Thanks, hope I can be there in this round.... please see my comment below. > I don't really understand the logic here. > You're rejecting the promise with EOS, but set mQueuedSample with mEOS to true so that the next call to GetSample will once again return EOS? > > What's the point? > > do you need mQueuedSample here? aNumSamples is >= 1 (currently we always pass 1) so if in 'aNumSamples==1' or 'there is no remaining sample in demuxer' condition, I should reject the promise with EOS error directly. in 'aNumSamples > 1' case, I should resolve the promise with preceding samples I got from this loop, then queue the EOS sample and wait for next GetSamples being called.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review150684 ::: dom/media/hls/HLSDemuxer.h:53 (Diff revisions 6 - 7) > > void NotifyDataArrived() override; > > AutoTaskQueue* GetTaskQueue() const { return mTaskQueue; } > void OnInitialized(bool aHasAudio, bool aHasVideo); > + class RefCountedMutex I'm not sure I see the point of making the mutex refcounted. It only makes the overall picture unecessarily complicated. if the HlsDemuxerCallback is itself refcounted, you can simply have a mutex there instead. ::: dom/media/hls/HLSDemuxer.cpp:90 (Diff revisions 6 - 7) > void OnInitialized(bool aHasAudio, bool aHasVideo) > { > - MOZ_ASSERT(mDemuxer); > - > + HLS_DEBUG("HlsDemuxerCallbacksSupport", > + "OnInitialized"); > + // Double checked locking for avoiding race condition. > + if (!mDemuxer) return; that first test is unnecessary ::: dom/media/hls/HLSDemuxer.cpp:96 (Diff revisions 6 - 7) > + MutexAutoLock lock(mRefMutex->mMutex); > + if (!mDemuxer) return; > + RefPtr<HlsDemuxerCallbacksSupport> self = this; > - mDemuxer->GetTaskQueue()->Dispatch(NS_NewRunnableFunction( > + mDemuxer->GetTaskQueue()->Dispatch(NS_NewRunnableFunction( > - [=] () { > + [=] () { > - mDemuxer->OnInitialized(aHasAudio, aHasVideo); > + MutexAutoLock lock(mRefMutex->mMutex); you know that self is kept from being destroyed. so just check take the mutex and test mDemuxer ::: dom/media/hls/HLSDemuxer.cpp:113 (Diff revisions 6 - 7) > "Got error(%d) from java side", > aErrorCode); > } > + void Detach() > + { > + mDemuxer = nullptr; must take the lock before accessing mDemuxer ::: dom/media/hls/HLSDemuxer.cpp:119 (Diff revisions 6 - 7) > + } > > private: > + ~HlsDemuxerCallbacksSupport() { } > HLSDemuxer* mDemuxer; > + RefPtr<HLSDemuxer::RefCountedMutex> mRefMutex; I would just have a non-mutable Mutex there. The callback is refcounted

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review150684 > I'm not sure I see the point of making the mutex refcounted. It only makes the overall picture unecessarily complicated. > > if the HlsDemuxerCallback is itself refcounted, you can simply have a mutex there instead. Thanks for the idea. I've used the mutex in the callback class itself. > I would just have a non-mutable Mutex there. The callback is refcounted I must use the mutable modifier since I will use this mutex in a const function.

Comment on attachment 8864783 [details] Bug 1350246 - [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. https://reviewboard.mozilla.org/r/136454/#review151164 ::: dom/media/hls/HLSDemuxer.cpp:78 (Diff revisions 7 - 8) > typedef GeckoHlsDemuxerWrapper::HlsDemuxerCallbacks::Natives<HlsDemuxerCallbacksSupport> NativeCallbacks; > using NativeCallbacks::DisposeNative; > using NativeCallbacks::AttachNative; > > - HlsDemuxerCallbacksSupport(HLSDemuxer* aDemuxer, > - HLSDemuxer::RefCountedMutex* aMutex) > + HlsDemuxerCallbacksSupport(HLSDemuxer* aDemuxer) > + : mMutex("HlsDemuxerCallbacksSupport"), mDemuxer(aDemuxer) one initialization per line please. ::: dom/media/hls/HLSDemuxer.cpp:88 (Diff revisions 7 - 8) > void OnInitialized(bool aHasAudio, bool aHasVideo) > { > HLS_DEBUG("HlsDemuxerCallbacksSupport", > "OnInitialized"); > - // Double checked locking for avoiding race condition. > - if (!mDemuxer) return; > + MutexAutoLock lock(mMutex); > + if (mDemuxer == nullptr) { return; } if (!mDemuxer) (per coding style guidelines https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#CC_practices "When testing a pointer, use (!myPtr) or (myPtr); don't use myPtr != nullptr or myPtr == nullptr."

Comment on attachment 8864781 [details] Bug 1350246 - [Part1] Implement a HLSDecoder as a MediaDecoder. https://reviewboard.mozilla.org/r/136450/#review139916 > As discussed, I don't think we should be using a HLSResource wrapper/object > > it's unecessary and only complicate the matter Yes, we should avoid passing resource instance to demuxer(but we need resource instance to get the GeckoHlsPlayer in current design). and we have a solution that Kilik did in Bug 1368954. we will decouple this.

Pushed by jacheng@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/0a728d8ee04e [Part0] Add a Util header for PR logging and define MOZ_ANDROID_HLS_SUPPORT macro. r=jya https://hg.mozilla.org/integration/autoland/rev/cc1756d3a157 [Part1] Implement a HLSDecoder as a MediaDecoder. r=jya https://hg.mozilla.org/integration/autoland/rev/3329c56140d8 [Part2] Implement a HLSResource as a MediaResource. r=jya https://hg.mozilla.org/integration/autoland/rev/dbada8f72a7c [Part3] Implement a HLSDemuxer and its trackdemuxers for AV. r=jya https://hg.mozilla.org/integration/autoland/rev/e3a9f5830afe [Part4] Add a feature preference for HLS, default on for Fennec. r=jya https://hg.mozilla.org/integration/autoland/rev/0183cff4d5c1 [Part5] Move IsHttpLiveStreamingType from DecoderTraits.cpp to its .h. and modify the callers related to this change. r=jya https://hg.mozilla.org/integration/autoland/rev/3b6c5d4d671c [Part6] Use DecoderTraits::IsHttpLiveStreamingType to implement HLSDecoder::IsSupportedType and use HLSDecoder::IsSupportedType to implement the following APIs for mimetype of HLS. r=jya https://hg.mozilla.org/integration/autoland/rev/7214ec83c310 [Part7] Create HLSResource if the mimetype is HLS. r=jya https://hg.mozilla.org/integration/autoland/rev/5b8f17ff8f92 [Part8] Add the native code implementation into source tree. r=jya

https://hg.mozilla.org/mozilla-central/rev/0a728d8ee04e https://hg.mozilla.org/mozilla-central/rev/cc1756d3a157 https://hg.mozilla.org/mozilla-central/rev/3329c56140d8 https://hg.mozilla.org/mozilla-central/rev/dbada8f72a7c https://hg.mozilla.org/mozilla-central/rev/e3a9f5830afe https://hg.mozilla.org/mozilla-central/rev/0183cff4d5c1 https://hg.mozilla.org/mozilla-central/rev/3b6c5d4d671c https://hg.mozilla.org/mozilla-central/rev/7214ec83c310 https://hg.mozilla.org/mozilla-central/rev/5b8f17ff8f92