We found some websites like[1], the audio source will change the format dynamically during playback. We should handle this case by changing the stream id to let MFR recreate audio decoder. Otherwise, the audio decoder will fire decoding error e.g. SoftAAC2: aacDecoder_DecodeFrame decoderErr = 0x4004 SoftAAC2: AAC decoder returned error 0x4004, substituting silence and the audio will be mute. [1] http://www.newshub.co.nz/home/new-zealand/2016/07/powerball-winners-cash-in-tickets.html

Hello jya, I would like consult that for audio AAC Is there any approach to prepend format information to a audio sample just like h.264 video that we can prepend SPS/PPS in the beginning of the sample? Current solution is to recreate the decoder when we got audio format change.... it seems the android audio decoder cannot tolerate the format change so we cannot reuse it. I just want to know instead of recreating if there is anyway to tell the decoder the changes? Thank you.

Comment on attachment 8885592 [details] Bug 1380237 - Tweak HLSDemuxer to handle audio format change. https://reviewboard.mozilla.org/r/156444/#review161564 ::: dom/media/hls/HLSDemuxer.cpp:23 (Diff revision 1) > using namespace mozilla::java; > > namespace mozilla { > > -static Atomic<uint32_t> sStreamSourceID(0u); > +static Atomic<uint32_t> sVideoStreamSourceID(0u); > +static Atomic<uint32_t> sAudioStreamSourceID(0u); use a common streamID, no need for two static atomics ::: dom/media/hls/HLSDemuxer.cpp:23 (Diff revision 1) > using namespace mozilla::java; > > namespace mozilla { > > -static Atomic<uint32_t> sStreamSourceID(0u); > +static Atomic<uint32_t> sVideoStreamSourceID(0u); > +static Atomic<uint32_t> sAudioStreamSourceID(0u); use a common ID for both audio and video, we only care that the value change over time. And it makes your code unecessarily complicated ::: dom/media/hls/HLSDemuxer.cpp:537 (Diff revision 1) > return nullptr; > } > > - // Update streamSouceID & videoInfo for MFR. > - if (mType == TrackInfo::kVideoTrack) { > + // Update A/V stream souce ID & Audio/VideoInfo for MFR. > + void (HLSDemuxer::*UpdateInfo)(int) = nullptr; > + Atomic<uint32_t>* sourceID = nullptr; this is way too complicated for what needs to be done. dont use a pointer to an atomic. Just copy the value in a uint32_t and increment the global ID. ::: dom/media/hls/HLSDemuxer.cpp:552 (Diff revision 1) > + } > - auto sampleFormatIndex = aSample->FormatIndex(); > + auto sampleFormatIndex = aSample->FormatIndex(); > - if (mLastFormatIndex != sampleFormatIndex) { > + if (mLastFormatIndex != sampleFormatIndex) { > - mLastFormatIndex = sampleFormatIndex; > + mLastFormatIndex = sampleFormatIndex; > - mParent->UpdateVideoInfo(mLastFormatIndex); > + (mParent->*UpdateInfo)(mLastFormatIndex); > - MutexAutoLock lock(mParent->mMutex); > + MutexAutoLock lock(mParent->mMutex); releasing the mutex when you take it right after doesn't make your code threadsafe. I find the entire logic with UpdateAudioInfo/UpdateVideoInfo only to create a new TrackInfoSharedPtr cumbersome and hard to follow. Having the parent own the VideoInfo and AudioInfo adds an extra layer of unnecessary complexity. Having the HLSTrackDemuxer use a TrackInfo instead would have been easier and remove the need for callbacks like this with mutexes. At the very least for now, don't use those pointers to atomics...

Comment on attachment 8885592 [details] Bug 1380237 - Tweak HLSDemuxer to handle audio format change. https://reviewboard.mozilla.org/r/156444/#review161976 ::: dom/media/hls/HLSDemuxer.h:104 (Diff revision 3) > bool GetSamplesMayBlock() const override > { > return false; > } > > + bool IsTrackValid() const { { on the next linst ::: dom/media/hls/HLSDemuxer.cpp:151 (Diff revision 3) > HLSDemuxer::OnInitialized(bool aHasAudio, bool aHasVideo) > { > MOZ_ASSERT(OnTaskQueue()); > > if (aHasAudio) { > - UpdateAudioInfo(0); > + mAudioDemuxer = new HLSTrackDemuxer(this, wouldn't it be simpler to let the HLSTrackDemuxer own the AudioInfo/VideoInfo? So create the TrackInfo in the HLSTrackDemuxer constructor and call UpdateMediaInfo(0) there. ::: dom/media/hls/HLSDemuxer.cpp:532 (Diff revision 3) > - if (mType == TrackInfo::kVideoTrack) { > - auto sampleFormatIndex = aSample->FormatIndex(); > + auto sampleFormatIndex = aSample->FormatIndex(); > - if (mLastFormatIndex != sampleFormatIndex) { > + if (mLastFormatIndex != sampleFormatIndex) { > - mLastFormatIndex = sampleFormatIndex; > + mLastFormatIndex = sampleFormatIndex; > - mParent->UpdateVideoInfo(mLastFormatIndex); > - MutexAutoLock lock(mParent->mMutex); > + UpdateMediaInfo(mLastFormatIndex); > + mrd->mTrackInfo = new TrackInfoSharedPtr(*mTrackInfo, ++sStreamSourceID); mTrackInfo accessed without holding the mutex. If it's intended, comment on the why....

Comment on attachment 8885592 [details] Bug 1380237 - Tweak HLSDemuxer to handle audio format change. https://reviewboard.mozilla.org/r/156444/#review161976 > wouldn't it be simpler to let the HLSTrackDemuxer own the AudioInfo/VideoInfo? > > So create the TrackInfo in the HLSTrackDemuxer constructor and call UpdateMediaInfo(0) there. Thanks. > mTrackInfo accessed without holding the mutex. > > If it's intended, comment on the why.... not intended, amended.

Pushed by jacheng@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/990004230604 Tweak HLSDemuxer to handle audio format change. r=jya