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 347504 f9dec37d375c018a587eb20a6ec958fa3d4c28e9
parent 347503 b1f32a906af2f3aaaf653d804f83ddf0f5884e31
child 347505 e49d5a02100e257e20cc9447633a8c8045c466f3
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgerald
bugs1288329
milestone50.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 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