Bug 1300296: P2. Don't rely on MP4 container to properly report if a frame is a keyframe. r=kentuckyfriedtakahe
authorJean-Yves Avenard <jyavenard@mozilla.com>
Sun, 04 Sep 2016 21:33:23 +1000
changeset 313106 06e25a05496fd9659767aaa9606cec61f5177a3b
parent 313105 d77a82f4a665eebed92e30a43a91152b4fc4b7aa
child 313107 d7e38eda81dba94dd26713db46de15e924d5d3eb
push id30671
push usercbook@mozilla.com
push dateThu, 08 Sep 2016 09:59:51 +0000
treeherdermozilla-central@bd28be90aed8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskentuckyfriedtakahe
bugs1300296
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1300296: P2. Don't rely on MP4 container to properly report if a frame is a keyframe. r=kentuckyfriedtakahe There are too many cases where the MP4 is improperly muxed and frames are incorrectly reported as keyframe. Instead we now look inside the H264 stream and check for IDR frames. We also ensure that the first frame returned after a seek is always a true keyframe. For plain MP4, seeking in those broken files will lead to broken A/V sync. The only way to fix this would be to check for the frame type when reading the samples table. However, this would require to read the entire stream which isn't viable. MozReview-Commit-ID: Cpv5y7HVD0N
dom/media/fmp4/MP4Demuxer.cpp
--- a/dom/media/fmp4/MP4Demuxer.cpp
+++ b/dom/media/fmp4/MP4Demuxer.cpp
@@ -9,29 +9,32 @@
 #include <stdint.h>
 
 #include "MP4Demuxer.h"
 #include "mp4_demuxer/MoofParser.h"
 #include "mp4_demuxer/MP4Metadata.h"
 #include "mp4_demuxer/ResourceStream.h"
 #include "mp4_demuxer/BufferStream.h"
 #include "mp4_demuxer/Index.h"
+#include "nsPrintfCString.h"
 
 // Used for telemetry
 #include "mozilla/Telemetry.h"
 #include "mp4_demuxer/AnnexB.h"
 #include "mp4_demuxer/H264.h"
 
 #include "nsAutoPtr.h"
 
 extern mozilla::LazyLogModule gMediaDemuxerLog;
 mozilla::LogModule* GetDemuxerLog() {
   return gMediaDemuxerLog;
 }
 
+#define LOG(arg, ...) MOZ_LOG(gMediaDemuxerLog, mozilla::LogLevel::Debug, ("MP4Demuxer(%p)::%s: " arg, this, __func__, ##__VA_ARGS__))
+
 namespace mozilla {
 
 class MP4TrackDemuxer : public MediaTrackDemuxer
 {
 public:
   MP4TrackDemuxer(MP4Demuxer* aParent,
                   UniquePtr<TrackInfo>&& aInfo,
                   const nsTArray<mp4_demuxer::Index::Indice>& indices);
@@ -50,29 +53,30 @@ public:
 
   media::TimeIntervals GetBuffered() override;
 
   void BreakCycles() override;
 
 private:
   friend class MP4Demuxer;
   void NotifyDataArrived();
-  void UpdateSamples(nsTArray<RefPtr<MediaRawData>>& aSamples);
+  already_AddRefed<MediaRawData> GetNextSample();
   void EnsureUpToDateIndex();
   void SetNextKeyFrameTime();
   RefPtr<MP4Demuxer> mParent;
   RefPtr<mp4_demuxer::ResourceStream> mStream;
   UniquePtr<TrackInfo> mInfo;
   RefPtr<mp4_demuxer::Index> mIndex;
   UniquePtr<mp4_demuxer::SampleIterator> mIterator;
   Maybe<media::TimeUnit> mNextKeyframeTime;
   // Queued samples extracted by the demuxer, but not yet returned.
   RefPtr<MediaRawData> mQueuedSample;
   bool mNeedReIndex;
   bool mNeedSPSForTelemetry;
+  bool mIsH264 = false;
 };
 
 
 // Returns true if no SPS was found and search for it should continue.
 bool
 AccumulateSPSTelemetry(const MediaByteBuffer* aExtradata)
 {
   mp4_demuxer::SPSData spsdata;
@@ -236,16 +240,17 @@ MP4TrackDemuxer::MP4TrackDemuxer(MP4Demu
 {
   EnsureUpToDateIndex(); // Force update of index
 
   VideoInfo* videoInfo = mInfo->GetAsVideoInfo();
   // Collect telemetry from h264 AVCC SPS.
   if (videoInfo &&
       (mInfo->mMimeType.EqualsLiteral("video/mp4") ||
        mInfo->mMimeType.EqualsLiteral("video/avc"))) {
+    mIsH264 = true;
     RefPtr<MediaByteBuffer> extraData = videoInfo->mExtraData;
     mNeedSPSForTelemetry = AccumulateSPSTelemetry(extraData);
     mp4_demuxer::SPSData spsdata;
     if (mp4_demuxer::H264::DecodeSPSFromExtraData(extraData, spsdata) &&
         spsdata.pic_width > 0 && spsdata.pic_height > 0 &&
         mp4_demuxer::H264::EnsureSPSIsSane(spsdata)) {
       videoInfo->mImage.width = spsdata.pic_width;
       videoInfo->mImage.height = spsdata.pic_height;
@@ -284,52 +289,127 @@ RefPtr<MP4TrackDemuxer::SeekPromise>
 MP4TrackDemuxer::Seek(media::TimeUnit aTime)
 {
   int64_t seekTime = aTime.ToMicroseconds();
   mQueuedSample = nullptr;
 
   mIterator->Seek(seekTime);
 
   // Check what time we actually seeked to.
-  mQueuedSample = mIterator->GetNext();
-  if (mQueuedSample) {
-    seekTime = mQueuedSample->mTime;
-  }
+  RefPtr<MediaRawData> sample;
+  do {
+    sample = GetNextSample();
+    if (!sample) {
+      return SeekPromise::CreateAndReject(DemuxerFailureReason::END_OF_STREAM, __func__);
+    }
+    if (!sample->Size()) {
+      // This sample can't be decoded, continue searching.
+      continue;
+    }
+    if (sample->mKeyframe) {
+      mQueuedSample = sample;
+      seekTime = mQueuedSample->mTime;
+    }
+  } while (!mQueuedSample);
+
   SetNextKeyFrameTime();
 
   return SeekPromise::CreateAndResolve(media::TimeUnit::FromMicroseconds(seekTime), __func__);
 }
 
+already_AddRefed<MediaRawData>
+MP4TrackDemuxer::GetNextSample()
+{
+  RefPtr<MediaRawData> sample = mIterator->GetNext();
+  if (!sample) {
+    return nullptr;
+  }
+  if (mInfo->GetAsVideoInfo()) {
+    sample->mExtraData = mInfo->GetAsVideoInfo()->mExtraData;
+    if (mIsH264) {
+      mp4_demuxer::H264::FrameType type =
+        mp4_demuxer::H264::GetFrameType(sample);
+      switch (type) {
+        case mp4_demuxer::H264::FrameType::I_FRAME: MOZ_FALLTHROUGH;
+        case mp4_demuxer::H264::FrameType::OTHER:
+        {
+          bool keyframe = type == mp4_demuxer::H264::FrameType::I_FRAME;
+          if (sample->mKeyframe != keyframe) {
+            NS_WARNING(nsPrintfCString("Frame incorrectly marked as %skeyframe @ pts:%lld dur:%u dts:%lld",
+                                       keyframe ? "" : "non-",
+                                       sample->mTime,
+                                       sample->mDuration,
+                                       sample->mTimecode).get());
+            sample->mKeyframe = keyframe;
+          }
+          break;
+        }
+        case mp4_demuxer::H264::FrameType::INVALID:
+          NS_WARNING(nsPrintfCString("Invalid H264 frame @ pts:%lld dur:%u dts:%lld",
+                                     sample->mTime,
+                                     sample->mDuration,
+                                     sample->mTimecode).get());
+          // We could reject the sample now, however demuxer errors are fatal.
+          // So we keep the invalid frame, relying on the H264 decoder to
+          // handle the error later.
+          // TODO: make demuxer errors non-fatal.
+          break;
+      }
+    }
+  }
+  if (sample->mCrypto.mValid) {
+    nsAutoPtr<MediaRawDataWriter> writer(sample->CreateWriter());
+    writer->mCrypto.mMode = mInfo->mCrypto.mMode;
+    writer->mCrypto.mIVSize = mInfo->mCrypto.mIVSize;
+    writer->mCrypto.mKeyId.AppendElements(mInfo->mCrypto.mKeyId);
+  }
+  return sample.forget();
+}
+
 RefPtr<MP4TrackDemuxer::SamplesPromise>
 MP4TrackDemuxer::GetSamples(int32_t aNumSamples)
 {
   EnsureUpToDateIndex();
   RefPtr<SamplesHolder> samples = new SamplesHolder;
   if (!aNumSamples) {
     return SamplesPromise::CreateAndReject(DemuxerFailureReason::DEMUXER_ERROR, __func__);
   }
 
   if (mQueuedSample) {
+    MOZ_ASSERT(mQueuedSample->mKeyframe,
+               "mQueuedSample must be a keyframe");
     samples->mSamples.AppendElement(mQueuedSample);
     mQueuedSample = nullptr;
     aNumSamples--;
   }
   RefPtr<MediaRawData> sample;
-  while (aNumSamples && (sample = mIterator->GetNext())) {
+  while (aNumSamples && (sample = GetNextSample())) {
     if (!sample->Size()) {
       continue;
     }
     samples->mSamples.AppendElement(sample);
     aNumSamples--;
   }
 
   if (samples->mSamples.IsEmpty()) {
     return SamplesPromise::CreateAndReject(DemuxerFailureReason::END_OF_STREAM, __func__);
   } else {
-    UpdateSamples(samples->mSamples);
+    for (const auto& sample : samples->mSamples) {
+      // Collect telemetry from h264 Annex B SPS.
+      if (mNeedSPSForTelemetry && mp4_demuxer::AnnexB::HasSPS(sample)) {
+        RefPtr<MediaByteBuffer> extradata =
+        mp4_demuxer::AnnexB::ExtractExtraData(sample);
+        mNeedSPSForTelemetry = AccumulateSPSTelemetry(extradata);
+      }
+    }
+
+    if (mNextKeyframeTime.isNothing() ||
+        samples->mSamples.LastElement()->mTime >= mNextKeyframeTime.value().ToMicroseconds()) {
+      SetNextKeyFrameTime();
+    }
     return SamplesPromise::CreateAndResolve(samples, __func__);
   }
 }
 
 void
 MP4TrackDemuxer::SetNextKeyFrameTime()
 {
   mNextKeyframeTime.reset();
@@ -344,43 +424,16 @@ void
 MP4TrackDemuxer::Reset()
 {
   mQueuedSample = nullptr;
   // TODO, Seek to first frame available, which isn't always 0.
   mIterator->Seek(0);
   SetNextKeyFrameTime();
 }
 
-void
-MP4TrackDemuxer::UpdateSamples(nsTArray<RefPtr<MediaRawData>>& aSamples)
-{
-  for (size_t i = 0; i < aSamples.Length(); i++) {
-    MediaRawData* sample = aSamples[i];
-    // Collect telemetry from h264 Annex B SPS.
-    if (mNeedSPSForTelemetry && mp4_demuxer::AnnexB::HasSPS(sample)) {
-      RefPtr<MediaByteBuffer> extradata =
-        mp4_demuxer::AnnexB::ExtractExtraData(sample);
-      mNeedSPSForTelemetry = AccumulateSPSTelemetry(extradata);
-    }
-    if (sample->mCrypto.mValid) {
-      nsAutoPtr<MediaRawDataWriter> writer(sample->CreateWriter());
-      writer->mCrypto.mMode = mInfo->mCrypto.mMode;
-      writer->mCrypto.mIVSize = mInfo->mCrypto.mIVSize;
-      writer->mCrypto.mKeyId.AppendElements(mInfo->mCrypto.mKeyId);
-    }
-    if (mInfo->GetAsVideoInfo()) {
-      sample->mExtraData = mInfo->GetAsVideoInfo()->mExtraData;
-    }
-  }
-  if (mNextKeyframeTime.isNothing() ||
-      aSamples.LastElement()->mTime >= mNextKeyframeTime.value().ToMicroseconds()) {
-    SetNextKeyFrameTime();
-  }
-}
-
 nsresult
 MP4TrackDemuxer::GetNextRandomAccessPoint(media::TimeUnit* aTime)
 {
   if (mNextKeyframeTime.isNothing()) {
     // There's no next key frame.
     *aTime =
       media::TimeUnit::FromMicroseconds(std::numeric_limits<int64_t>::max());
   } else {
@@ -392,17 +445,17 @@ MP4TrackDemuxer::GetNextRandomAccessPoin
 RefPtr<MP4TrackDemuxer::SkipAccessPointPromise>
 MP4TrackDemuxer::SkipToNextRandomAccessPoint(media::TimeUnit aTimeThreshold)
 {
   mQueuedSample = nullptr;
   // Loop until we reach the next keyframe after the threshold.
   uint32_t parsed = 0;
   bool found = false;
   RefPtr<MediaRawData> sample;
-  while (!found && (sample = mIterator->GetNext())) {
+  while (!found && (sample = GetNextSample())) {
     parsed++;
     if (sample->mKeyframe && sample->mTime >= aTimeThreshold.ToMicroseconds()) {
       found = true;
       mQueuedSample = sample;
     }
   }
   SetNextKeyFrameTime();
   if (found) {
@@ -436,8 +489,10 @@ MP4TrackDemuxer::NotifyDataArrived()
 
 void
 MP4TrackDemuxer::BreakCycles()
 {
   mParent = nullptr;
 }
 
 } // namespace mozilla
+
+#undef LOG