Bug 1288329: [ogg] P3. Never take into considerations frames prior the first keyframe. r=gerald
authorJean-Yves Avenard <jyavenard@mozilla.com>
Fri, 29 Jul 2016 10:38:36 +1000
changeset 332538 f9dec37d375c018a587eb20a6ec958fa3d4c28e9
parent 332537 b1f32a906af2f3aaaf653d804f83ddf0f5884e31
child 332539 e49d5a02100e257e20cc9447633a8c8045c466f3
push id9858
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 14:37:10 +0000
treeherdermozilla-aurora@203106ef6cb6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1288329
milestone50.0a1
Bug 1288329: [ogg] P3. Never take into considerations frames prior the first keyframe. r=gerald MozReview-Commit-ID: 1aJSbJb9kQW
dom/media/ogg/OggDemuxer.cpp
dom/media/ogg/OggDemuxer.h
--- a/dom/media/ogg/OggDemuxer.cpp
+++ b/dom/media/ogg/OggDemuxer.cpp
@@ -185,42 +185,39 @@ OggDemuxer::HaveStartTime()
 const
 {
   return mStartTime.isSome();
 }
 
 int64_t
 OggDemuxer::StartTime() const
 {
-  MOZ_ASSERT(HaveStartTime());
-  return mStartTime.ref();
+  return mStartTime.refOr(0);
 }
 
 bool
 OggDemuxer::HaveStartTime(TrackInfo::TrackType aType)
 {
-  return (aType == TrackInfo::kAudioTrack ? mAudioOggState : mVideoOggState)
-           .mStartTime.isSome();
+  return OggState(aType).mStartTime.isSome();
 }
 
 int64_t
 OggDemuxer::StartTime(TrackInfo::TrackType aType)
 {
-  return (aType == TrackInfo::kAudioTrack ? mAudioOggState : mVideoOggState)
-           .mStartTime.refOr(TimeUnit::FromMicroseconds(0)).ToMicroseconds();
+  return OggState(aType).mStartTime.refOr(TimeUnit::FromMicroseconds(0)).ToMicroseconds();
 }
 
 RefPtr<OggDemuxer::InitPromise>
 OggDemuxer::Init()
 {
-  int ret = ogg_sync_init(OggState(TrackInfo::kAudioTrack));
+  int ret = ogg_sync_init(OggSyncState(TrackInfo::kAudioTrack));
   if (ret != 0) {
     return InitPromise::CreateAndReject(DemuxerFailureReason::DEMUXER_ERROR, __func__);
   }
-  ret = ogg_sync_init(OggState(TrackInfo::kVideoTrack));
+  ret = ogg_sync_init(OggSyncState(TrackInfo::kVideoTrack));
   if (ret != 0) {
     return InitPromise::CreateAndReject(DemuxerFailureReason::DEMUXER_ERROR, __func__);
   }
   /*
   if (InitBufferedState() != NS_OK) {
     return InitPromise::CreateAndReject(DemuxerFailureReason::WAITING_FOR_DATA, __func__);
   }
   */
@@ -310,21 +307,22 @@ OggDemuxer::GetTrackDemuxer(TrackInfo::T
 
   return e.forget();
 }
 
 nsresult
 OggDemuxer::Reset(TrackInfo::TrackType aType)
 {
   // Discard any previously buffered packets/pages.
-  ogg_sync_reset(OggState(aType));
+  ogg_sync_reset(OggSyncState(aType));
   OggCodecState* trackState = GetTrackCodecState(aType);
   if (trackState) {
     return trackState->Reset();
   }
+  OggState(aType).mNeedKeyframe = true;
   return NS_OK;
 }
 
 bool
 OggDemuxer::ReadHeaders(TrackInfo::TrackType aType,
                         OggCodecState* aState,
                         OggHeaders& aHeaders)
 {
@@ -671,19 +669,18 @@ OggDemuxer::ReadMetadata()
   }
 
   SetupTargetSkeleton();
   SetupMediaTracksInfo(serials);
 
   if (HasAudio() || HasVideo()) {
     int64_t startTime = -1;
     FindStartTime(startTime);
-    NS_ASSERTION(startTime >= 0, "Must have a non-negative start time");
-    OGG_DEBUG("Detected stream start time %lld", startTime);
     if (startTime >= 0) {
+      OGG_DEBUG("Detected stream start time %lld", startTime);
       mStartTime.emplace(startTime);
     }
 
     if (mInfo.mMetadataDuration.isNothing() &&
         Resource(TrackInfo::kAudioTrack)->GetLength() >= 0 &&
         Resource(TrackInfo::kAudioTrack)->GetResource()->IsTransportSeekable()) {
       // We didn't get a duration from the index or a Content-Duration header.
       // Seek to the end of file to find the end time.
@@ -835,67 +832,70 @@ OggDemuxer::ReadOggChain(const media::Ti
                       nsAutoPtr<MediaInfo>(new MediaInfo(mInfo))));
     }
     return true;
   }
 
   return false;
 }
 
-ogg_sync_state*
+OggDemuxer::OggStateContext&
 OggDemuxer::OggState(TrackInfo::TrackType aType)
 {
   if (aType == TrackInfo::kVideoTrack) {
-    return &mVideoOggState.mOggState.mState;
+    return mVideoOggState;
   }
-  return &mAudioOggState.mOggState.mState;
+  return mAudioOggState;
+}
+
+ogg_sync_state*
+OggDemuxer::OggSyncState(TrackInfo::TrackType aType)
+{
+  return &OggState(aType).mOggState.mState;
 }
 
 MediaResourceIndex*
 OggDemuxer::Resource(TrackInfo::TrackType aType)
 {
-  if (aType == TrackInfo::kVideoTrack) {
-    return &mVideoOggState.mResource;
-  }
-  return &mAudioOggState.mResource;
+  return &OggState(aType).mResource;
 }
 
 MediaResourceIndex*
 OggDemuxer::CommonResource()
 {
   return &mAudioOggState.mResource;
 }
 
 bool
 OggDemuxer::ReadOggPage(TrackInfo::TrackType aType, ogg_page* aPage)
 {
   int ret = 0;
-  while((ret = ogg_sync_pageseek(OggState(aType), aPage)) <= 0) {
+  while((ret = ogg_sync_pageseek(OggSyncState(aType), aPage)) <= 0) {
     if (ret < 0) {
       // Lost page sync, have to skip up to next page.
       continue;
     }
     // Returns a buffer that can be written too
     // with the given size. This buffer is stored
     // in the ogg synchronisation structure.
-    char* buffer = ogg_sync_buffer(OggState(aType), 4096);
+    char* buffer = ogg_sync_buffer(OggSyncState(aType), 4096);
     NS_ASSERTION(buffer, "ogg_sync_buffer failed");
 
     // Read from the resource into the buffer
     uint32_t bytesRead = 0;
 
     nsresult rv = Resource(aType)->Read(buffer, 4096, &bytesRead);
     if (NS_FAILED(rv) || !bytesRead) {
       // End of file or error.
       return false;
     }
 
     // Update the synchronisation layer with the number
     // of bytes written to the buffer
-    ret = ogg_sync_wrote(OggState(aType), bytesRead);
+    ret = ogg_sync_wrote(OggSyncState(aType), bytesRead);
     NS_ENSURE_TRUE(ret == 0, false);
   }
 
   return true;
 }
 
 nsresult
 OggDemuxer::DemuxOggPage(TrackInfo::TrackType aType, ogg_page* aPage)
@@ -933,25 +933,37 @@ OggDemuxer::GetCrypto()
   return nullptr;
 }
 
 ogg_packet*
 OggDemuxer::GetNextPacket(TrackInfo::TrackType aType)
 {
   OggCodecState* state = GetTrackCodecState(aType);
   ogg_packet* packet = nullptr;
+  OggStateContext& context = OggState(aType);
 
-  do {
+  while (true) {
     if (packet) {
       OggCodecState::ReleasePacket(state->PacketOut());
     }
     DemuxUntilPacketAvailable(aType, state);
 
     packet = state->PacketPeek();
-  } while (packet && state->IsHeader(packet));
+    if (!packet) {
+      break;
+    }
+    if (state->IsHeader(packet)) {
+      continue;
+    }
+    if (context.mNeedKeyframe && !state->IsKeyframe(packet)) {
+      continue;
+    }
+    context.mNeedKeyframe = false;
+    break;
+  }
 
   return packet;
 }
 
 void
 OggDemuxer::DemuxUntilPacketAvailable(TrackInfo::TrackType aType,
                                       OggCodecState* aState)
 {
@@ -1134,19 +1146,19 @@ OggDemuxer::SeekInternal(TrackInfo::Trac
   nsresult res;
   int64_t adjustedTarget = target;
   int64_t startTime = StartTime(aType);
   int64_t endTime = mInfo.mMetadataDuration->ToMicroseconds();
   if (aType == TrackInfo::kAudioTrack && mOpusState){
     adjustedTarget = std::max(startTime, target - OGG_SEEK_OPUS_PREROLL);
   }
 
-  if (adjustedTarget == startTime) {
-    // We've seeked to the media start. Just seek to the offset of the first
-    // content page.
+  if (!HaveStartTime(aType) || adjustedTarget == startTime) {
+    // We've seeked to the media start or we can't seek.
+    // Just seek to the offset of the first content page.
     res = Resource(aType)->Seek(nsISeekableStream::NS_SEEK_SET, 0);
     NS_ENSURE_SUCCESS(res,res);
 
     res = Reset(aType);
     NS_ENSURE_SUCCESS(res,res);
   } else {
     // TODO: This may seek back unnecessarily far in the video, but we don't
     // have a way of asking Skeleton to seek to a different target for each
@@ -1271,17 +1283,17 @@ OggDemuxer::SeekToKeyframeUsingIndex(Tra
   res = Reset(aType);
   NS_ENSURE_SUCCESS(res, SEEK_FATAL_ERROR);
 
   // Check that the page the index thinks is exactly here is actually exactly
   // here. If not, the index is invalid.
   ogg_page page;
   int skippedBytes = 0;
   PageSyncResult syncres = PageSync(Resource(aType),
-                                    OggState(aType),
+                                    OggSyncState(aType),
                                     false,
                                     keyframe.mKeyPoint.mOffset,
                                     Resource(aType)->GetLength(),
                                     &page,
                                     skippedBytes);
   NS_ENSURE_TRUE(syncres != PAGE_SYNC_ERROR, SEEK_FATAL_ERROR);
   if (syncres != PAGE_SYNC_OK || skippedBytes != 0) {
     LOG(LogLevel::Debug, ("Indexed-seek failure: Ogg Skeleton Index is invalid "
@@ -1954,17 +1966,17 @@ OggDemuxer::SeekBisection(TrackInfo::Tra
       previousGuess = guess;
 
       hops++;
 
       // Locate the next page after our seek guess, and then figure out the
       // granule time of the audio and video bitstreams there. We can then
       // make a bisection decision based on our location in the media.
       PageSyncResult pageSyncResult = PageSync(Resource(aType),
-                                               OggState(aType),
+                                               OggSyncState(aType),
                                                false,
                                                guess,
                                                endOffset,
                                                &page,
                                                skippedBytes);
       NS_ENSURE_TRUE(pageSyncResult != PAGE_SYNC_ERROR, NS_ERROR_FAILURE);
 
       if (pageSyncResult == PAGE_SYNC_END_OF_RANGE) {
--- a/dom/media/ogg/OggDemuxer.h
+++ b/dom/media/ogg/OggDemuxer.h
@@ -269,24 +269,28 @@ private:
   // contructor was called. We can't check it dynamically because
   // we're not on the main thread;
   bool mOpusEnabled;
 
   // Decode state of the Skeleton bitstream.
   SkeletonState* mSkeletonState;
 
   // Ogg decoding state.
-  struct OggStateContext {
-    explicit OggStateContext(MediaResource* aResource) : mResource(aResource) {}
+  struct OggStateContext
+  {
+    explicit OggStateContext(MediaResource* aResource)
+    : mResource(aResource), mNeedKeyframe(true) {}
     nsAutoOggSyncState mOggState;
     MediaResourceIndex mResource;
     Maybe<media::TimeUnit> mStartTime;
+    bool mNeedKeyframe;
   };
 
-  ogg_sync_state* OggState(TrackInfo::TrackType aType);
+  OggStateContext& OggState(TrackInfo::TrackType aType);
+  ogg_sync_state* OggSyncState(TrackInfo::TrackType aType);
   MediaResourceIndex* Resource(TrackInfo::TrackType aType);
   MediaResourceIndex* CommonResource();
   OggStateContext mAudioOggState;
   OggStateContext mVideoOggState;
 
   // Vorbis/Opus/Theora data used to compute timestamps. This is written on the
   // decoder thread and read on the main thread. All reading on the main
   // thread must be done after metadataloaded. We can't use the existing