Closed Bug 1171257 Opened 9 years ago Closed 9 years ago

[Aries][Video] Video recorded on device has short pauses while playing back in video app

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla42
Project Flags:
blocking-b2g 2.5+
Tracking Flags:
Tracking Status
firefox42 --- fixed
b2g-master --- verified

People

(Reporter: bzumwalt, Assigned: sotaro)

References

(
URL
)

Details

(Keywords: smoketest, Whiteboard: [3.0-Daily-Testing],[spark])

Attachments

(3 files, 11 obsolete files)

Logcat
314.25 KB, text/plain
Details
patch part2 - Change to MediaPromise.h
2.50 KB, patch
bholley
: review+
Details | Diff | Splinter Review
patch part1 - Add force decode ahead to MediaFormatReader
25.45 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
Attached file Logcat — 
Description:
While viewing video recorded on device using video app, the video playback "pauses" repeatedly giving the video a choppy appearance.

Repro Steps:
1) Update a Xperia Z3 Compact (B2G) to 20150603164854
2) Take ~10 second video with camera
3) Launch video and play movie recorded in step 2

Actual:
Video has intermitant pauses.

Expected:
Video playback is smooth with no pauses.

Environmental Variables:
Device: Xperia Z3 Compact (B2G) 3.0
Build ID: 20150603164854
Gaia: ff80db87926a5c2769e158801090465b4ed117fa
Gecko: 196d99aabc27
Gonk: Could not pull gonk.  Did you shallow Flash?
Version: 41.0a1 (3.0)

Repro frequency: 3/3
See attached: Youtube video clip & logcat
Youtube link: http://youtu.be/nTJqdA3v1Wo
Failing case: https://moztrap.mozilla.org/manage/case/2478/

Issue does NOT reproduce on Flame 3.0

Video playback is smooth with no pauses.

Device: Flame 2.2
Build ID: 20150603002503
Gaia: a9aeb08263f1a727136e8ae78425e52431c82770
Gecko: 5b3f1796ddf6
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
[Blocking Requested - why for this release]:
Regression from Flame behavior that fails smoke tests.

Failing case: https://moztrap.mozilla.org/manage/case/2478/
blocking-b2g: --- → spark?
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
The problem might happen because of gonk media's(framework/av) implementation. gonk media needs modification for b2g if we want to get optimum performance.
triage team: blocking
blocking-b2g: spark? → spark+
Sotaro, can you investigate this please?
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #5)
> Sotaro, can you investigate this please?

I do not have Xperia Z3 Compact device. Therefore, I could not investigate. My assumption is comment 3. Is there a way to get the device?
Flags: needinfo?(sotaro.ikeda.g) → needinfo?(drs)
We've arranged offline to have a Z3C provided for you, Sotaro. Please assign this to yourself when you start working on it. Thanks.
Flags: needinfo?(drs) → needinfo?(sotaro.ikeda.g)
Okey, Thanks.
Flags: needinfo?(sotaro.ikeda.g)
Flags: needinfo?(sotaro.ikeda.g)
Assignee: nobody → sotaro.ikeda.g
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Sotaro Ikeda [:sotaro] from comment #3)
> The problem might happen because of gonk media's(framework/av)
> implementation. gonk media needs modification for b2g if we want to get
> optimum performance.

I checked the above on Z3C. stagefright was moz built binary(not default binary) and MediaCodec and ACodec have a correct fix of Bug 1009410.
I checked a Z3C recorded video. The video was 3840*2160 30fps. This high resolution video seems to cause the problem.
On master, platform independent MP4Reader is used for mp4(3gp) playback. It also seems to affect to the problem. When MediaOmxReader is used for mp4 playback, the pause problem seems to happen.

Therefore gonk's MP4Reader implementation seems not provide enough performance compared to MediaOmxReader. Bug 1170589 seems like a similar bug.
See Also: → 1170589
When I did not plug usb cable, the problem seems happen often on MP4Reader's playback.

The problem seems not happen on MediaOmxReader and MediaCodecReader. Therefore, gonk's MP4Reader might have a problem.
Component: Gaia::Video → Video/Audio
Product: Firefox OS → Core
Sotaro, did you want us to try the patches, yet?
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #15)
> Sotaro, did you want us to try the patches, yet?

They are just to check for each MediaReaders' capability. Anyway we need to continue to use MP4Reader.
Flags: needinfo?(sotaro.ikeda.g)
(In reply to Sotaro Ikeda [:sotaro] from comment #12)
> When I did not plug usb cable, the problem seems happen often on MP4Reader's
> playback.
> 
> The problem seems not happen on MediaOmxReader and MediaCodecReader.
> Therefore, gonk's MP4Reader might have a problem.

Correction:
On master, MediaFormatReader is enabled instead of MP4Reader.

Result of summary.
- MediaFormatReader(default): Problem happen
- MP4Reader: Problem happen
- MediaCodecReader: Problem does not happen
- MediaOmxReader: Problem does not happen

One wired thing is that when the phone is charged via usb cable, the problem seems not happen.
The following seems to affect to the problem.
- When usb is not plugged, cpu clock is slow compared to plugged situation.
- MediaDecoderStateMachine's ample video queue size is 3 on gonk.
   On gonk, video buffer is very scarce resource.
- 3840*2160 video's decoding take long time(demux and decode)
- MediaFormatReader uses a lot of dispatch for decoding.
MediaFormatReader's video decoding sequence is like the following. 

MediaFormatReader::RequestVideoData()
->MediaFormatReader::ScheduleUpdate(TrackInfo::kVideoTrack)
->MediaFormatReader::GetTaskQueue()->Dispatch(task.forget());
// schedule (MediaFormatReader task thread)
MediaFormatReader::Update()
 ->MediaFormatReader::RequestDemuxSamples()
  ->MediaFormatReader::DoDemuxVideo()
 ->MediaFormatReader::DecodeDemuxedSamples()
  ->MP4TrackDemuxer::GetSamples(1)
// schedule (MediaFormatReader task thread)
 ->MediaFormatReader::OnVideoDemuxCompleted()
  ->mVideo.mQueuedSamples.AppendElements(aSamples->mSamples);
  ->MediaFormatReader::ScheduleUpdate(TrackInfo::kVideoTrack);
// schedule (MediaFormatReader task thread)
MediaFormatReader::Update()
 ->MediaFormatReader::RequestDemuxSamples()
  ->MediaFormatReader::DoDemuxVideo()
 ->MediaFormatReader::DecodeDemuxedSamples()
  ->GonkMediaDataDecoder::Input()
   ->GonkMediaDataDecoder::mTaskQueue->Dispatch()
// schedule (VideoTaskQueue thread)
GonkMediaDataDecoder::ProcessDecode()
 ->GonkVideoDecoderManager::Input()
  ->MediaCodecProxy::Input();
 ->GonkMediaDataDecoder::ProcessOutput()
  ->GonkVideoDecoderManager::Output()
   ->MediaCodecProxy::Output()
 ->MediaFormatReader::Output()
// schedule (MediaFormatReader task thread)
->MediaFormatReader::NotifyNewOutput()
 ->MediaFormatReader::ScheduleUpdate(aTrack);
// schedule (MediaFormatReader task thread)
->MediaFormatReader::Update()
 ->MediaFormatReader::ReturnOutput()
  ->MediaFormatReader::mVideo.mPromise.Resolve()
(In reply to Sotaro Ikeda [:sotaro] from comment #18)
> The following seems to affect to the problem.
> - When usb is not plugged, cpu clock is slow compared to plugged situation.
> - MediaDecoderStateMachine's ample video queue size is 3 on gonk.
>    On gonk, video buffer is very scarce resource.
> - 3840*2160 video's decoding take long time(demux and decode)
> - MediaFormatReader uses a lot of dispatch for decoding.

If we can increase the number of buffer, the problem could be fixed. But we could not choose it on gonk. MediaOmxReader already works correctly current number of buffers. MediaFormatReader also needs to works correctly with same amount of video buffers.
MediaFormatReader seems to lost cpu time between each RequestVideoData().
(In reply to Sotaro Ikeda [:sotaro] from comment #21)
> MediaFormatReader seems to lost cpu time between each RequestVideoData().

When decoding time is almost same to playback speed. It causes the problem.
When short pause happened, SkipVideoDemuxToNextKeyFrame() was called in MediaFormatReader::RequestVideoData().
https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaFormatReader.cpp#545

This video frame skip caused short pause.
The patch eagerly decode video frame even when promise does not exist.

This fixes the problem. Therefore this eagerness seems to make the symptom difference like Comment 17.
Comment on attachment 8620748 [details] [diff] [review]
wip patch - Eagerly decode ahead video on gonk

:jya, what is a good way to more eagerly decode video frames on gonk?
Attachment #8620748 - Flags: feedback?(jyavenard)
(In reply to Sotaro Ikeda [:sotaro] from comment #25)
> Comment on attachment 8620748 [details] [diff] [review]
> wip patch - Eagerly decode ahead video on gonk
> 
> :jya, what is a good way to more eagerly decode video frames on gonk?

I don't like it as it's be worked around in that patch.

There's everything in place to decode more than one frame ahead with the media.video-decode-ahead/media.audio-decode-ahead. Except that this is currently a misnomer as that value isn't use to calculate how many "decode" ahead we will perform, but instead it limits how many frames we will demux if the decoder doesn't call "InputExhausted". As all decoders always call InputExhausted() after every call to MediaDataDecoder::Input(), the media.*-decode-ahead is in practice ignored.

What I suggest instead, is that we use this value to really determine how many frames we will be decoding ahead of a call to RequestAudioData/RequestVideoData.

This can be enabled for all platforms, not just gonk and they will all benefit from it.

So we would not only decode ahead, but also demux ahead.
Chris, what do you think?
Flags: needinfo?(cpearce)
The problem seems to happen by the followings
- [1] At most one MediaPromise exist for video decoding.
- [2] There is a time between last video decode and and next MediaFormatReader::RequestVideoData() call.
- [3] 3840*2160 size video's decoding speed seems almost same as playback speed.
- [4] time of [2] seems to cause slow decoding speed than playback speed.
(In reply to Sotaro Ikeda [:sotaro] from comment #27)
> - [4] time of [2] seems to cause slow decoding speed than playback speed.

have you been able to run a profiler ?

because all my profiler reports, that *by far*, (and by far I mean several orders of magnitude) decoding and rendering time dwarf the overhead related to starting a new task and media promise dispatch.

The fact that you're having the issue with MP4Reader too, which doesn't make use of promises to the extent MediaFormatreader does (the MP4Reader always resolves immediately the RequestVideoData promise; while MediaFormatReader will queue a task to demux, queue a task to process the result, queue a task to feed the decoder, get the decoded frame, queue a task to process the decoded frame and queue a task to return the frame back to the MDSM).

Yet, due to how MediaFormatReader caches things like buffered ranges which is access constantly by the MDSM (every frame), MediaFormatReader uses significantly less CPU than the old MP4Reader. On the same 1 minute playback, MP4Reader would use 34s of CPU time, while MediaFormatReader uses 21s.
Recalculating the buffered range is rather heavy due to how the MoofParser rescan everything and that NotifyDataArrived is called every 32kB of downloaded data (which causes a refresh of the MP4Demuxer cache)

Decoding multiple frames ahead, may improve the situation slightly, but if the decoder is too slow.. well it's too slow.

I wouldn't expect a handled device to be able to decode a 4K video. Not even desktop PC can do it properly (and we had to blacklist 4K on all AMD video cards)
(In reply to Jean-Yves Avenard [:jya] from comment #28)
> 
> I wouldn't expect a handled device to be able to decode a 4K video. Not even
> desktop PC can do it properly (and we had to blacklist 4K on all AMD video
> cards)

Decoding is done by hw codec and the following site says that xperia z3 compact supports 4K video's recording and playback. And MediaOMXReader could playback the video.
  http://www.sonymobile.com/global-en/products/phones/xperia-z3-compact/features/#camera
(In reply to Jean-Yves Avenard [:jya] from comment #28)
> have you been able to run a profiler ?

I did not take a profiler. I think it does not work well to fix the problem, because cpu usage is not high. The actual decoding is done by hw. The following is the result of "adb shell top" command during the recorded video's playback.

-----------------------------------------------------------
User 29%, System 25%, IOW 0%, IRQ 0%
User 313 + Nice 0 + Sys 267 + Idle 475 + IOW 3 + IRQ 0 + SIRQ 3 = 1061

  PID   TID PR CPU% S     VSS     RSS PCY UID      Thread          Proc
  293  1801  2  11% R 184672K  53636K  fg media    OMXCallbackDisp /system/bin/mediaserver
 1596  1815  2   6% S 296080K  96152K  fg u0_a1596 MediaPD~oder #3 /system/b2g/plugin-container
   52    52  0   4% S      0K      0K     root     system          
 1596  1803  2   4% S 296080K  95888K  fg u0_a1596 MediaPD~oder #2 /system/b2g/plugin-container
 1596  1802  0   3% R 296080K  95888K  fg u0_a1596 MediaPD~oder #1 /system/b2g/plugin-container
 1596  1879  2   1% S 296080K  96152K  fg u0_a1596 MediaPD~oder #4 /system/b2g/plugin-container
  317   958  2   1% S 338828K  96012K     root     Compositor      /system/b2g/b2g
This is not gonk specific patch. This also fixed the problem on z3c.
Fix nits.
Attachment #8620986 - Attachment is obsolete: true
Attachment #8620748 - Flags: feedback?(jyavenard)
Comment on attachment 8621012 [details] [diff] [review]
wip patch - Force decode ahead when video frames are scarce

:jya, can you feedback to the patch? The patch tried to fix the problem by a platform independent way.
Attachment #8621012 - Flags: feedback?(jyavenard)
Comment on attachment 8621012 [details] [diff] [review]
wip patch - Force decode ahead when video frames are scarce

Review of attachment 8621012 [details] [diff] [review]:
-----------------------------------------------------------------

That's much better than the earlier proposal.
Though, maybe it would be cleaner to make a mirror VideoQueue().GetSize() in the MediaDecoderReader (or just in MediaFormatReader even).
This is related to the skip to next keyframe logic and I believe cpearce's long term goal is to have the calculation performed in the reader rather than the MDSM. this would move us into that direction.

I still believe that changing the meaning of the media.video-decode-ahead preference is the way to go (and would achieve the same goal here) ; where media.video-decode-ahead actually indicates how many frames ahead will be decoded (not just fed to the decoder).
You can then tweak that preference per platform.

The new demuxer API allows to demux more than one frame at a time. So we could demux 2 or more frame at once, and feed them directly to the decoder. Each following call to RequestVideoData will immediately get one of the decoded frame from existing pool of decoded frames..

But if the change is urgent, I'm happy with you moving ahead with that change now and I'll revisit it once I'm done with my MSE rewrite.

::: dom/media/MediaFormatReader.cpp
@@ +727,5 @@
> +      !aDecoder.mError &&
> +      !aDecoder.mDemuxRequest.Exists() &&
> +      !aDecoder.mDemuxEOS &&
> +      aDecoder.mOutput.IsEmpty()) {
> +    return true;

return
    !aDecoder.mError &&
    (aDecoder.HasPromise() || aDecoder.mForceDecodeAhead) &&
    !aDecoder.mDemuxRequest.Exists() &&
    aDecoder.mOutput.IsEmpty() &&
    (aDecoder.mInputExhausted || !aDecoder.mQueuedSamples.IsEmpty() ||
     Decoder.mForceDecodeAhead ||
     aDecoder.mNumSamplesInput - aDecoder.mNumSamplesOutput < aDecoder.mDecodeAhead);
 below
Attachment #8621012 - Flags: feedback?(cpearce)
Comment on attachment 8621012 [details] [diff] [review]
wip patch - Force decode ahead when video frames are scarce

Review of attachment 8621012 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/MediaPromise.h
@@ +921,5 @@
> +  Arg1Type mArg1;
> +  Arg2Type mArg2;
> +  Arg3Type mArg3;
> +};
> +

this should be done in a separate patch, and preferably with variadic templates (you can check NS_NewRunnableMethodWithArgs)
(In reply to Jean-Yves Avenard [:jya] from comment #34)
> The new demuxer API allows to demux more than one frame at a time. So we
> could demux 2 or more frame at once, and feed them directly to the decoder.
> Each following call to RequestVideoData will immediately get one of the
> decoded frame from existing pool of decoded frames..
> 
> But if the change is urgent, I'm happy with you moving ahead with that
> change now and I'll revisit it once I'm done with my MSE rewrite.

Thanks for the quick feedback! We need to fix this bug as soon as possible as part of Spark project.

https://wiki.mozilla.org/Firefox_OS/spark
Then in this bug, I am going to create a patch as a short term fix.
Attachment #8620748 - Attachment is obsolete: true
(In reply to Jean-Yves Avenard [:jya] from comment #35)
> Comment on attachment 8621012 [details] [diff] [review]
> wip patch - Force decode ahead when video frames are scarce
> 
> Review of attachment 8621012 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/MediaPromise.h
> @@ +921,5 @@
> > +  Arg1Type mArg1;
> > +  Arg2Type mArg2;
> > +  Arg3Type mArg3;
> > +};
> > +
> 
> this should be done in a separate patch, and preferably with variadic
> templates (you can check NS_NewRunnableMethodWithArgs)

Thanks for info. But I wanted variadic templates to out of scope this bug.

        Attachment #8621012 -
        Attachment is obsolete: true

        Attachment #8621012 -
        Flags: feedback?(jyavenard)

        Attachment #8621012 -
        Flags: feedback?(cpearce)

    
  

        Attachment #8621638 -
        Attachment description: patch - Add force decode ahead to MediaFormatReader → patch part1 - Add force decode ahead to MediaFormatReader

    
  

        Attachment #8621638 -
        Flags: review?(jyavenard)

        Attachment #8621638 -
        Flags: review?(cpearce)

    
  

        Attachment #8621639 -
        Flags: review?(bobbyholley)

    
    

    
  


  
Re-based.

        Attachment #8621638 -
        Attachment is obsolete: true

        Attachment #8621638 -
        Flags: review?(jyavenard)

        Attachment #8621638 -
        Flags: review?(cpearce)

    
  

        Attachment #8621861 -
        Flags: review?(jyavenard)

        Attachment #8621861 -
        Flags: review?(cpearce)

    
    

    
  
Comment on attachment 8621861 [details] [diff] [review]
patch part1 - Add force decode ahead to MediaFormatReader

Review of attachment 8621861 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/MediaFormatReader.cpp
@@ +1364,5 @@
>    }
>    if (mVideo.mDecoder) {
>      mVideo.mDecoder->Shutdown();
>      mVideo.mDecoder = nullptr;
> +    mVideo.mForceDecodeAhead = false;

You want this to be in DecoderData::ResetState

::: dom/media/MediaFormatReader.h
@@ +216,5 @@
>      nsAutoPtr<DecoderCallback> mCallback;
>  
>      // Only accessed from reader's task queue.
>      uint32_t mDecodeAhead;
> +    bool mForceDecodeAhead;

set mForceDecodeAhead to false in ResetState()

::: dom/media/fmp4/MP4Reader.cpp
@@ +609,5 @@
>  
>  nsRefPtr<MediaDecoderReader::VideoDataPromise>
>  MP4Reader::RequestVideoData(bool aSkipToNextKeyframe,
> +                            int64_t aTimeThreshold,
> +                            bool aForceDecodeAhead)

anything done in MediaFormatReader needs to be done in MP4Reader (until the time it's removed) as the logic is almost identical for this.

::: dom/media/mediasource/MediaSourceReader.cpp
@@ +361,5 @@
>  
>  void
>  MediaSourceReader::DoVideoRequest()
>  {
> +  mVideoRequest.Begin(GetVideoReader()->RequestVideoData(mDropVideoBeforeThreshold, GetReaderVideoTime(mTimeThreshold), false)

If you have issues playing this 4K video, then you will have issue playing 4K MSE too.
You need to store the value of aForceDecodeAhead and pass it to the sub-readers.

        Attachment #8621861 -
        Flags: review?(jyavenard) → review-

    
    

    
  


  
Apply the comments.

        Attachment #8621861 -
        Attachment is obsolete: true

        Attachment #8621861 -
        Flags: review?(cpearce)

        Attachment #8621639 -
        Flags: review?(bobbyholley) → review+

    
  

        Attachment #8622569 -
        Flags: review?(jyavenard)

        Attachment #8622569 -
        Flags: review?(cpearce)

        Attachment #8622569 -
        Flags: review?(jyavenard) → review+

    
    

    
  
Comment on attachment 8622569 [details] [diff] [review]
patch part1 - Add force decode ahead to MediaFormatReader

Review of attachment 8622569 [details] [diff] [review]:
-----------------------------------------------------------------

Long term, using media.video-decode-ahead for this would be better, as jya suggested.

::: dom/media/MediaDecoderStateMachine.cpp
@@ +1997,5 @@
>    }
>  
>    bool skipToNextKeyFrame = NeedToSkipToNextKeyframe();
>    int64_t currentTime = mState == DECODER_STATE_SEEKING ? 0 : GetMediaTime();
> +  bool forceDecodeAhead = static_cast<uint32_t>(VideoQueue().GetSize()) <= GetScarceVideoFrames();

Why add a GetScarceVideoFrames() function? Why not just make this:

  bool forceDecodeAhead = static_cast<uint32_t>(VideoQueue().GetSize()) <= SCARCE_VIDEO_QUEUE_SIZE;

??? It seems that this is the only place that GetScarceVideoFrames() is used, so it seems unnecessary to make it a function?

        Attachment #8622569 -
        Flags: review?(cpearce) → review+
Flags: needinfo?(cpearce)

    
    

    
  
(In reply to Chris Pearce (:cpearce) from comment #44)
> Comment on attachment 8622569 [details] [diff] [review]
> patch part1 - Add force decode ahead to MediaFormatReader
> 
> Review of attachment 8622569 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Long term, using media.video-decode-ahead for this would be better, as jya
> suggested.

I agree.

> 
> ::: dom/media/MediaDecoderStateMachine.cpp
> @@ +1997,5 @@
> >    }
> >  
> >    bool skipToNextKeyFrame = NeedToSkipToNextKeyframe();
> >    int64_t currentTime = mState == DECODER_STATE_SEEKING ? 0 : GetMediaTime();
> > +  bool forceDecodeAhead = static_cast<uint32_t>(VideoQueue().GetSize()) <= GetScarceVideoFrames();
> 
> Why add a GetScarceVideoFrames() function? Why not just make this:
> 
>   bool forceDecodeAhead = static_cast<uint32_t>(VideoQueue().GetSize()) <=
> SCARCE_VIDEO_QUEUE_SIZE;
> 
> ??? It seems that this is the only place that GetScarceVideoFrames() is
> used, so it seems unnecessary to make it a function?

I'll update in a next patch.

    
    

    
  


  
Apply the comment. Carry "r=jya,cpearce".

        Attachment #8622569 -
        Attachment is obsolete: true

    
    

    
  
(In reply to Sotaro Ikeda [:sotaro] from comment #47)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=06fe7cc7c497

some test failures on mse and eme tests on b2g and mac.

    
    

    
  
(In reply to Sotaro Ikeda [:sotaro] from comment #48)
> (In reply to Sotaro Ikeda [:sotaro] from comment #47)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=06fe7cc7c497
> 
> some test failures on mse and eme tests on b2g and mac.

hmmm... so the decoder attempts to continue decoding even after the GMP decoder has been shutdown.
Previously, the MDSM would ensure that only one decode would be happening and has completed before destroying the GMP reader.

Need to have a mechanism to tell the reader to stop decoding ahead.

    
    

    
  


  
Rebased.

        Attachment #8623215 -
        Attachment is obsolete: true

        Attachment #8625791 -
        Flags: review+

    
  

        Attachment #8617495 -
        Attachment is obsolete: true

    
  

        Attachment #8617496 -
        Attachment is obsolete: true
blocking-b2g: spark+ → 2.5+

    
    

    
  


  
This is a short time fix of EMEDecryptor::Drain(). The Drain() implementations seems to have a problem. After calling it, there are cases that EMEDecryptor::Decrypted() is called. This seems timing dependent problem of EMEDecryptor.

For a long term, more correct fix should replace it.

    
  

        Attachment #8625897 -
        Attachment is obsolete: true

    
  

        Attachment #8628795 -
        Flags: review?(cpearce)

    
    

    
  
Comment on attachment 8628795 [details] [diff] [review]
partch part 3 - Fix EMEDecryptor::Drain()

Review of attachment 8628795 [details] [diff] [review]:
-----------------------------------------------------------------

I think this could still fail if we send input again before receiving the output from a previous decrypt arrives back from the GMP process.

We should just use MediaPromises for CDMProxy::Decrypt, and disconnect them when we flush or drain. I have a patch; I'll file a new bug and make it block this bug.

        Attachment #8628795 -
        Flags: review?(cpearce) → review-

    
    

    
  
(In reply to Chris Pearce (:cpearce) from comment #57)
> We should just use MediaPromises for CDMProxy::Decrypt, and disconnect them
> when we flush or drain. I have a patch; I'll file a new bug and make it
> block this bug.

Patch up in bug 1180070. Sotaro, please verify this fixes your bug. Thanks!

    
  

        Attachment #8628795 -
        Attachment is obsolete: true

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/43ffa51ed029
Status: NEW → RESOLVED
Closed: 9 years ago

          status-firefox42:
          --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42

    
    

    
  
This issue is verified fixed on the latest Spark Aries 2.5 build.
Video playback is consistent and smooth.

Environmental Variables:
Device: Aries 2.5
Build ID: 20150706103023
Gaia: dc6c18c0dea7af3c40bfff86c530fd877d899dc4
Gecko: cef11c3e86c3
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 42.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:42.0) Gecko/42.0 Firefox/42.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]

          status-b2g-master:
          affected → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Blocks: 1197075

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: