Bug 1195094: P2. Ensure TrackInfo object passed to constructor is never modified. r=cpearce
authorJean-Yves Avenard <jyavenard@mozilla.com>
Thu, 29 Oct 2015 00:46:31 +1100
changeset 305056 b4401440ad833c2b6cea09e3ef589fba28cc2f0a
parent 305055 bb0870b54aefc3a5b42070cb66fa68114bf36896
child 305057 3bb8446a6d8ddb5970e584a81312e4e07aeb331d
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
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 1195094: P2. Ensure TrackInfo object passed to constructor is never modified. r=cpearce The PDM documentation states that it is safe to keep a reference to the TrackInfo object provided to the constructor. However, this wasn't enforced by the H264Converter which would modify the object and worse, did in a non thread-safe fashion
--- a/dom/media/platforms/PlatformDecoderModule.h
+++ b/dom/media/platforms/PlatformDecoderModule.h
@@ -209,16 +209,18 @@ public:
   // Decoder needs to decide whether or not hardware accelearation is supported
   // after creating. It doesn't need to call Init() before calling this function.
   virtual bool IsHardwareAccelerated(nsACString& aFailureReason) const { return false; }
   // ConfigurationChanged will be called to inform the video or audio decoder
   // that the format of the next input sample is about to change.
   // If video decoder, aConfig will be a VideoInfo object.
   // If audio decoder, aConfig will be a AudioInfo object.
+  // It is not safe to store a reference to this object and the decoder must
+  // make a copy.
   virtual nsresult ConfigurationChanged(const TrackInfo& aConfig)
     return NS_OK;
 } // namespace mozilla
--- a/dom/media/platforms/wrappers/H264Converter.cpp
+++ b/dom/media/platforms/wrappers/H264Converter.cpp
@@ -17,16 +17,17 @@ namespace mozilla
 H264Converter::H264Converter(PlatformDecoderModule* aPDM,
                              const VideoInfo& aConfig,
                              layers::LayersBackend aLayersBackend,
                              layers::ImageContainer* aImageContainer,
                              FlushableTaskQueue* aVideoTaskQueue,
                              MediaDataDecoderCallback* aCallback)
   : mPDM(aPDM)
+  , mOriginalConfig(aConfig)
   , mCurrentConfig(aConfig)
   , mLayersBackend(aLayersBackend)
   , mImageContainer(aImageContainer)
   , mVideoTaskQueue(aVideoTaskQueue)
   , mCallback(aCallback)
   , mDecoder(nullptr)
   , mNeedAVCC(aPDM->DecoderNeedsConversion(aConfig) == PlatformDecoderModule::kNeedAVCC)
   , mLastError(NS_OK)
@@ -134,17 +135,17 @@ nsresult
   if (mNeedAVCC && !mp4_demuxer::AnnexB::HasSPS(mCurrentConfig.mExtraData)) {
     // nothing found yet, will try again later
-  mDecoder = mPDM->CreateVideoDecoder(mCurrentConfig,
+  mDecoder = mPDM->CreateVideoDecoder(mNeedAVCC ? mCurrentConfig : mOriginalConfig,
   if (!mDecoder) {
     mLastError = NS_ERROR_FAILURE;
     return NS_ERROR_FAILURE;
--- a/dom/media/platforms/wrappers/H264Converter.h
+++ b/dom/media/platforms/wrappers/H264Converter.h
@@ -48,16 +48,17 @@ private:
   nsresult CreateDecoderAndInit(MediaRawData* aSample);
   nsresult CheckForSPSChange(MediaRawData* aSample);
   void UpdateConfigFromExtraData(MediaByteBuffer* aExtraData);
   void OnDecoderInitDone(const TrackType aTrackType);
   void OnDecoderInitFailed(MediaDataDecoder::DecoderFailureReason aReason);
   RefPtr<PlatformDecoderModule> mPDM;
+  const VideoInfo& mOriginalConfig;
   VideoInfo mCurrentConfig;
   layers::LayersBackend mLayersBackend;
   RefPtr<layers::ImageContainer> mImageContainer;
   RefPtr<FlushableTaskQueue> mVideoTaskQueue;
   nsTArray<RefPtr<MediaRawData>> mMediaRawSamples;
   MediaDataDecoderCallback* mCallback;
   RefPtr<MediaDataDecoder> mDecoder;
   MozPromiseRequestHolder<InitPromise> mInitPromiseRequest;