Closed
Bug 877461
Opened 11 years ago
Closed 11 years ago
[Intermittent] YouTube - Video freezes (audio continues to play)
Categories
(Firefox OS Graveyard :: General, defect)
Tracking
(blocking-b2g:leo+, firefox22 wontfix, firefox23 wontfix, firefox24 fixed, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)
People
(Reporter: jsmith, Assigned: roc)
References
Details
(Whiteboard: [tef-triage][YouTubeCertBlocker+])
Attachments
(4 files, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
cpearce
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
cajbir
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
Saw this three times.
Example STR #1
1. Started playing to tell a lie episode (~40 min long) in YouTube certified app
2. One minute in - youtube video frozen, audio kept playing
Example STR #2
1. Was watching a 3 minute video in fullscreen and seeked into the future
2. Video began buffering, then I exited fullscreen
3. Audio kept playing, but the video did not
Reporter | ||
Comment 1•11 years ago
|
||
Filed a separate bug away from bug 876843 until we have exact confirmation that the YouTube certified app bug here is exactly the same as what was seen with viewing YouTube videos in the Video app. It might be the same though.
blocking-b2g: --- → tef?
Summary: [Intermittent] YouTube - Audio continues to play with video frozen → [Intermittent] YouTube - Video freezes (audio continues to play)
Sounds like this is either a media issue or a gfx issue.
Though, in honesty, I wonder if we're getting to the point where there's simply too many issues in the v1.0 codebase and we simply need to let it go and instead aim for v1.1 to be the first release that supports youtube :(
The fact that we're running in to so many fundamental issues is really concerning.
Comment 4•11 years ago
|
||
Is this the same as bug 871753?
Comment 5•11 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #4)
> Is this the same as bug 871753?
It is different from bug 871753. In bug 871753's case, audio and app's ui also freeze because of deadlock.
Comment 6•11 years ago
|
||
I tend to agree with comment 3, I think it is too late to try to fix this for 1.0.1 and we should go for do this properly for 1.1
Whiteboard: [tef-triage]
Reporter | ||
Comment 7•11 years ago
|
||
(In reply to Daniel Coloma:dcoloma from comment #6)
> I tend to agree with comment 3, I think it is too late to try to fix this
> for 1.0.1 and we should go for do this properly for 1.1
That would require a decision to pull the YouTube app permanently for 1.01 entirely. Which we haven't reached that decision just yet - we said we would make a final decision here at the latest by June 4th.
I tend to think though this will probably cause us to fail YouTube's certification process, even though it's intermittent across this bug and bug 876843 (i.e. we need to be solid on the quality bar when doing anything involving playing media content).
Comment 8•11 years ago
|
||
Jeff, let's see if we can work with Sotaro on this and shed some light on it.
Flags: needinfo?(jmuizelaar)
Comment 9•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Daniel Coloma:dcoloma from comment #6)
> > I tend to agree with comment 3, I think it is too late to try to fix this
> > for 1.0.1 and we should go for do this properly for 1.1
>
> That would require a decision to pull the YouTube app permanently for 1.01
> entirely. Which we haven't reached that decision just yet - we said we would
> make a final decision here at the latest by June 4th.
Or that we don't need this bug to ship YT. Have you or anybody else been able to reproduce since? What was the video used?
Keywords: qawanted
Comment 10•11 years ago
|
||
Able to reproduce 1 time out of 5 times on the
Unagi Build ID: 20130530070208
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/09ac1fd2959c
Gaia: 1cca9324d4444ad28c6fa99875e17abf7e8230be
Keywords: qawanted
Comment 11•11 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #0)
> Example STR #2
>
> 1. Was watching a 3 minute video in fullscreen and seeked into the future
> 2. Video began buffering, then I exited fullscreen
> 3. Audio kept playing, but the video did not
I can reproduce the bug intermittently using these STR. It seems that nsMediaOmxReader::DecodeVideoData() is returning false, which shuts down playback of the video stream but not the audio stream. nsMediaOmxReader::DecodeVideoData() is returning false because we're unable to create the the VideoData object (we're hitting the line NS_WARNING("Unable to create VideoData");, I'm not sure why yet.
Comment 12•11 years ago
|
||
We're unable to create the VideoData because OmxDecoder::ReadVideo is returning true but not filling out valid data in aFrame. mVideoSource->Read() is failing with err == -ETIMEDOUT.
Comment 13•11 years ago
|
||
I'm seeing this in the logcat right before we hit the -ETIMEDOUT return:
E/OMXCodec( 6033): [OMX.qcom.video.decoder.avc] Timed out waiting for output buffers: 0/12
Seems that something is holding onto the gralloc buffer too long.
Assignee | ||
Comment 14•11 years ago
|
||
This patch will recycle the ImageLayers used by recreated nsVideoFrames. It shouldn't make a difference (maybe be a bit more efficient), but it might help avoid some bugs?
Comment 15•11 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #13)
> I'm seeing this in the logcat right before we hit the -ETIMEDOUT return:
>
> E/OMXCodec( 6033): [OMX.qcom.video.decoder.avc] Timed out waiting for output
> buffers: 0/12
>
> Seems that something is holding onto the gralloc buffer too long.
It happens normally when MediaStreamSource::readAt() is blocked longer time.
http://mxr.mozilla.org/mozilla-b2g18/source/content/media/omx/OmxDecoder.cpp#95
Assignee | ||
Comment 16•11 years ago
|
||
This patch basically seems to work.
Assignee | ||
Comment 17•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #15)
> (In reply to Chris Pearce (:cpearce) from comment #13)
> > I'm seeing this in the logcat right before we hit the -ETIMEDOUT return:
> >
> > E/OMXCodec( 6033): [OMX.qcom.video.decoder.avc] Timed out waiting for output
> > buffers: 0/12
> >
> > Seems that something is holding onto the gralloc buffer too long.
>
> It happens normally when MediaStreamSource::readAt() is blocked longer time.
> http://mxr.mozilla.org/mozilla-b2g18/source/content/media/omx/OmxDecoder.
> cpp#95
Can you explain that in more detail? If readAt is blocked, wouldn't that cause the decoder to stop consuming buffers, thus making it more likely we *don't* run out of buffers?
Comment 18•11 years ago
|
||
I wrote comment #15, because it is the sympton I saw in the past. It need to make clear why.
OMXCodec uses one mutex to protect everything of OMXCodec. If MediaStreamSource::readAt() takes longer time, following things happen.
- Client can not read decoded data.
- buffers can not be returned to OMXCodec.
- thread in OMXCodec can not run
Assignee | ||
Comment 19•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #16)
> This patch basically seems to work.
I take that back, I don't think it does.
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #18)
> I wrote comment #15, because it is the sympton I saw in the past. It need to
> make clear why.
> OMXCodec uses one mutex to protect everything of OMXCodec. If
> MediaStreamSource::readAt() takes longer time, following things happen.
> - Client can not read decoded data.
> - buffers can not be returned to OMXCodec.
> - thread in OMXCodec can not run
OK, but if buffers aren't being returned to OMXCodec, and it's not consuming any, why would we run out?
Assignee | ||
Comment 21•11 years ago
|
||
I've tried pretty hard to reproduce this with a Gecko debug build, without success.
Reporter | ||
Comment 23•11 years ago
|
||
FWIW, I got this reproduce again. My STR this time was something along the lines of merely seeking ahead to a portion of the video that has not buffered yet with HQ disabled.
Reporter | ||
Updated•11 years ago
|
Comment 24•11 years ago
|
||
roc - what can QA do to get debugging when they're able to reproduce?
We've had a number of instances of this (sometimes within a couple of video plays). We should investigate as if it's a YT blocker.
Assignee: nobody → roc
blocking-b2g: tef? → tef+
Comment 26•11 years ago
|
||
batch update on tef+ milestones. partner to make a final on 6/3 Asia time. TEF+ needs to be resolved by 6/3 to be in the final build. thanks
Target Milestone: --- → 1.0.1 IOT3 (3jun)
Reporter | ||
Updated•11 years ago
|
QA Contact: jsmith
Comment 27•11 years ago
|
||
blocking-b2g: tef+ → leo+
Target Milestone: 1.0.1 IOT3 (3jun) → ---
Reporter | ||
Updated•11 years ago
|
status-b2g18:
--- → affected
status-b2g18-v1.0.0:
--- → wontfix
status-b2g18-v1.0.1:
--- → wontfix
status-b2g-v1.1hd:
--- → affected
Reporter | ||
Comment 28•11 years ago
|
||
Since we're not caring about 1.01 anymore, I found a reproducible STR that seems to always reproduce this bug with the HTML 5 YouTube app.
STR
1. Go to https://m.youtube.com/?tsp=1&player=html5&monetize=1 in the browser
2. Search for Community
3. Select "Community - Best of Troy (Season 1)
4. Disable the HQ button
5. Play the video
6. Seek forward in the video by a minute or two
Result - You'll see this bug.
Keywords: steps-wanted
Updated•11 years ago
|
Target Milestone: --- → 1.1 QE2 (6jun)
Comment 29•11 years ago
|
||
On browser youtube I see this message when the screen freezes
E/libgenlock( 132): perform_lock_unlock_operation: GENLOCK_IOC_DREADLOCK failed (lockType0x1, >err=Connection timed out fd=31)
E/omx_vdec( 132): Failed to acquire genlock, ret = 1
I you're seeing the "[OMX.qcom.video.decoder.avc] Timed out" message make sure you have the patches from bug 863441 and bug 864230.
Comment 30•11 years ago
|
||
(In reply to Diego Wilson [:diego] from comment #29)
> On browser youtube I see this message when the screen freezes
>
> E/libgenlock( 132): perform_lock_unlock_operation: GENLOCK_IOC_DREADLOCK
> failed (lockType0x1, >err=Connection timed out fd=31)
> E/omx_vdec( 132): Failed to acquire genlock, ret = 1
IIRC, I see this error only when hw composer rendering is enabled.
Comment 31•11 years ago
|
||
(In reply to Chris Pearce (:cpearce) from comment #13)
> I'm seeing this in the logcat right before we hit the -ETIMEDOUT return:
>
> E/OMXCodec( 6033): [OMX.qcom.video.decoder.avc] Timed out waiting for output
> buffers: 0/12
I checked the buri v1-train source code. In the code, when timeout happened in OMXCodec::read(), OMXCodec's state change to ERROR state. But such code does not present in AOSP. And also unagi v1-train source code does not have it.
Comment 32•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #31)
> I checked the buri v1-train source code. In the code, when timeout happened
> in OMXCodec::read(), OMXCodec's state change to ERROR state. But such code
> does not present in AOSP. And also unagi v1-train source code does not have
> it.
Is the libstagefright code for buri different to that in our mozilla-b2g/B2G repository? If so, how does one get access to that?
Assignee | ||
Comment 33•11 years ago
|
||
Sotaro, can you help us understand what is going on here?
What does it mean when the OMXCodec reports "Timed out waiting for output buffers"?
Is it a bug for MediaStreamSource::readAt to block indefinitely? Because the design of the media cache is that reads are allowed to block.
If we return an error, or zero bytes, instead of blocking, would that help?
How exactly can a blocking read from the cache cause the OMXCodec to report "Timed out waiting for output buffers"?
Comment 34•11 years ago
|
||
After adapting patch from Bug 870564, This problem is occurred.
I saw this log also.
E/OMXCodec( 6033): [OMX.qcom.video.decoder.avc] Timed out waiting for output buffers: 0/12
Comment 35•11 years ago
|
||
(In reply to Chris Double (:doublec) from comment #32)
> Is the libstagefright code for buri different to that in our mozilla-b2g/B2G
> repository? If so, how does one get access to that?
unagi(inari), hamachi(buri), leo use CAF(codeaurora)'s stagefright code. It is modifed from default android source.
It is defined in https://github.com/mozilla-b2g/b2g-manifest
Comment 36•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #33)
> What does it mean when the OMXCodec reports "Timed out waiting for output buffers"?
This error log is printed when OMXCodec can not provide decoded data within specified time(3 sec).
> Is it a bug for MediaStreamSource::readAt to block indefinitely? Because the design of the media cache is that reads are allowed to block.
read blocking is not a problem. stagefright works like that in android.
The problem is read is blocked very longer time than android.
And media cache size and others(like AMPLE_AUDIO_USECS) might affected to it.
> If we return an error, or zero bytes, instead of blocking, would that help?
It does not help in this case:-( OMXCodec expect that data source can be read same as local file system.
> How exactly can a blocking read from the cache cause the OMXCodec to report "Timed out waiting for output buffers"?
I did not investigated about it deeply. I am going to investigate about it.
Assignee | ||
Comment 38•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #36)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #33)
> > What does it mean when the OMXCodec reports "Timed out waiting for output buffers"?
>
> This error log is printed when OMXCodec can not provide decoded data within
> specified time(3 sec).
If the codec tries to read data we haven't fetched yet, then it could easily take more than 3 seconds to fetch. Especially if we have to initiate a new HTTP request to get the right byte range. Maybe the problem is as simple as that.
If that's the problem, then maybe we just need to have that timeout increased to a much larger value?
Comment 39•11 years ago
|
||
In android, 3 sec is enough for timeout. If media buffer works as expected this problem does not happen. I am wondering that current Firefox OS has some problem around handling MediaCache. If there are not enough pre-fetched data in MediaCache, decoding/playback should be paused. If this works correctly, this timeout problem does not happen.
In android, when prefetched data becomes low water mark, playback is paused. It is in AwesomePlayer::onBufferingUpdate()
http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/AwesomePlayer.cpp#692
Media cache is handled in NuCachedSource2 class.
http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/NuCachedSource2.cpp
One big different about media chache between FirefoxOS and android is that NuCachedSource2 keeps data continuous area. Firefox OS's media cache does not keep cache continuous area. In Firefox OS, during playback, audio cache read ahead 1sec from video track(this need to be changed to more shorter). If media cache caches data continuous area, video track have already data for decoding. But is is not guarantee in Firefox OS. Sometimes MediaCache does not have data for video track.
And this case is not handled in MediaOmxReader::GetBuffered()
http://mxr.mozilla.org/mozilla-central/source/content/media/omx/MediaOmxReader.cpp#289
Then new http session triggered by OMXCodec::read();
So, Need to fix part is around MediaCache, I think.
Comment 40•11 years ago
|
||
This timeout is used also bad content check that the hardware can not handle.
There was a bug to extend timeout, Bug 864181. But I set to WORKSFORME. Because to extend timeout cause bad effect to this case. And extend the timeout did not solve the problem.
Comment 41•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #39)
> In android, 3 sec is enough for timeout. If media buffer works as expected
> this problem does not happen.
It seems that an easier way to debug this is revert Bug 863441.
Assignee | ||
Comment 42•11 years ago
|
||
I think you're saying that we should only call into the OMX code to decode frames when we know all the data that it might read is already present in the media cache. But that seems very difficult to guarantee, since we don't know how much data it is going to need.
We can try to tweak our buffering heuristics so we buffer more before calling into the codec. But I don't think we can guarantee that the codec never runs out of cached data.
It looks like AwesomePlayer uses heuristics too. It seems to me that AwesomePlayer could also make the codec read uncached data if those heuristics are wrong; is that correct? Is there something I can't see that makes AwesomePlayer avoid these timeouts we're seeing?
Assignee | ||
Comment 43•11 years ago
|
||
Also, I don't understand why increasing the timeout would not fix this bug.
> And this case is not handled in MediaOmxReader::GetBuffered()
> http://mxr.mozilla.org/mozilla-central/source/content/media/omx/MediaOmxReader.cpp#289
GetBuffered is a heuristic (assumes constant bit rate), so I hope we're not depending on that to be very accurate.
I'll see if I can do something to deal with this audio/video skew issue, anyway.
Comment 44•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #43)
> Also, I don't understand why increasing the timeout would not fix this bug.
If network connection is cellular data connection, data disconnect period could be way much longer than wlan. We can not extend the timeout to fix this case. Extend timeout causes following drawbacks.
If we increase the timeout, OMXCodec::read() could be blocked until this timeout at most. It means, even when video tag try to release video decoder, the video tag could continue to hold hw codec the timeout period at most. This could happen in following use cases.
- a video content is corrupted
- device does not support a video content
- network connection becomes wrong
For example, we extend the timeout to 20 sec
If Sd card has 40 videos that does not supported to playback on the device. In this case, thumbnail processing could becomes 20*40 sec.
Another example. When a video is played in browser, user push home button and changed to video app. But at that time, there is a problem on the network connection. When a browser go to background, a hardware codec is going to be freed. The free could take 20 sec at most. Until the free end, the video app can not do video playback.
Comment 45•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #42)
> It looks like AwesomePlayer uses heuristics too. It seems to me that
> AwesomePlayer could also make the codec read uncached data if those
> heuristics are wrong; is that correct? Is there something I can't see that
> makes AwesomePlayer avoid these timeouts we're seeing?
In actual use, AwesomePlayer avoid this problem in almost all use cases. It is not only this heuristics, but also how caching is done in NuCachedSource2.
Comment 46•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #39)
> In android, 3 sec is enough for timeout. If media buffer works as expected
> this problem does not happen.
Correction, 3 sec since got OMXCodec's mutex. If mutex is already held by other task like reading data source, this duration is not counted.
http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/OMXCodec.cpp#3897
Comment 47•11 years ago
|
||
I found HwcComposer bug 878977. If I skip rendering canvas layers as described here https://bugzilla.mozilla.org/show_bug.cgi?id=878977#c1 my video doesn't lock anymore!
Of course, skipping canvas layers isn't an option, but it narrows down for me.
Flags: needinfo?(dwilson)
Comment 48•11 years ago
|
||
(In reply to Diego Wilson [:diego] from comment #47)
> I found HwcComposer bug 878977. If I skip rendering canvas layers as
> described here https://bugzilla.mozilla.org/show_bug.cgi?id=878977#c1 my
> video doesn't lock anymore!
>
> Of course, skipping canvas layers isn't an option, but it narrows down for
> me.
nice! Though I am not sure this fix all this Video freezes (audio continues to play) problem.
Assignee | ||
Comment 49•11 years ago
|
||
This bug shows up quickly with the steps in comment #28 because our seeking code doesn't do any buffering. All we do in nsMediaOmxReader::Seek is set mAudioSeekTimeUs and mVideoSeekTimeUs and call nsBuiltinDecoderReader::DecodeToTarget to decode video and audio up to the desired point. That in turn calls OmxDecoder::ReadVideo which simply does options.setSeekTo and then mVideoSource->read. Of course we don't have the data for that point in time, and can't get it in 3 seconds, so we get the timeout error from libstagefright.
This is difficult to fix well because this part of our code, like the rest of our code, is designed around the assumption that it's OK to make codecs block until the data they request is available. The libstagefright design, where we don't know what data the codec will request but apparently it has to be available immediately, seems crazy to me. But it is what it is I guess.
I don't see anything in AwesomePlayer that would avoid this sort of problem. It just sets the seek option and starts reading from the decoder without preloading any data or checking buffers. Sotaro, do you know how AwesomePlayer avoids a timeout when it seeks?
http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/AwesomePlayer.cpp#1600
Flags: needinfo?(sotaro.ikeda.g)
Assignee | ||
Comment 50•11 years ago
|
||
I tried completely disabling HwcComposer2D by adding a "return false;" at the start of HwcComposer2D::TryRender, and it fixed the seeking problem. The codec still does a lot of reads of uncached data but the ETIMEDOUT error is never returned. That makes me think that blocking the codec during a data read doesn't have to be a problem.
So I'm going to focus on 878977 next.
Assignee | ||
Comment 51•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #50)
> I tried completely disabling HwcComposer2D by adding a "return false;" at
> the start of HwcComposer2D::TryRender, and it fixed the seeking problem.
Actually it makes the bug harder to reproduce, but it still can occur. So never mind.
Assignee | ||
Comment 52•11 years ago
|
||
Attachment #757919 -
Flags: review?(cpearce)
Assignee | ||
Comment 53•11 years ago
|
||
I still don't know how to fix seeking. When we try to seek to a particular time, we don't even know which bytes of the resource are going to be needed by the codecs until they call readAt, and then it's too late because we've only got 3 seconds to fetch the requested data.
Comment 54•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #49)
>
> I don't see anything in AwesomePlayer that would avoid this sort of problem.
> It just sets the seek option and starts reading from the decoder without
> preloading any data or checking buffers. Sotaro, do you know how
> AwesomePlayer avoids a timeout when it seeks?
> http://androidxref.com/4.0.4/xref/frameworks/base/media/libstagefright/
> AwesomePlayer.cpp#1600
Actually AwesomePlayer does not fix seek problem. It does not make a problem in most use cases. But there is still a problem. Then Android is going to use NuPlayer for streaming playback. It fixes this problem. In JB, the NuPlayer is used for RTSP and HTTP Live Streaming.
http://androidxref.com/4.2.2_r1/xref/frameworks/av/media/libmediaplayerservice/MediaPlayerFactory.cpp#198
And seemsto use NuPlayer as default player in tuture.
http://androidxref.com/4.2.2_r1/xref/frameworks/av/media/libmediaplayerservice/MediaPlayerFactory.cpp#63
Flags: needinfo?(sotaro.ikeda.g)
Comment 55•11 years ago
|
||
NuPlayer does seek asynchronously. At first it does seek only by using parser without using OMXCodec.
http://androidxref.com/4.2.2_r1/xref/frameworks/av/media/libmediaplayerservice/nuplayer/NuPlayerDriver.cpp#184
Comment 56•11 years ago
|
||
Comment on attachment 757919 [details] [diff] [review]
Make nsMediaCache aggressively cache data just behind the current playback position
Review of attachment 757919 [details] [diff] [review]:
-----------------------------------------------------------------
::: content/media/nsMediaCache.cpp
@@ +932,1 @@
> BlockOwner* bo = &block->mOwners[i];
While you're here, how about you fix the "Blocks can be belong" typo on line 923?
Attachment #757919 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 57•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #53)
> I still don't know how to fix seeking. When we try to seek to a particular
> time, we don't even know which bytes of the resource are going to be needed
> by the codecs until they call readAt, and then it's too late because we've
> only got 3 seconds to fetch the requested data.
My current best idea for fixing seeking is to handle the timeout error by restarting decoding. If I can get restarting to work properly (which may require reinstantiating the libstagefright decoder, but hopefully not) then this should work pretty well --- there will still be a delay of course, but only as long as it takes the data to arrive, and since you're seeking that's expected.
Reporter | ||
Updated•11 years ago
|
Whiteboard: [tef-triage] → [tef-triage][YouTubeCertBlocker+]
Comment 58•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #57)
> My current best idea for fixing seeking is to handle the timeout error by
> restarting decoding. If I can get restarting to work properly (which may
> require reinstantiating the libstagefright decoder, but hopefully not) then
> this should work pretty well --- there will still be a delay of course, but
> only as long as it takes the data to arrive, and since you're seeking that's
> expected.
I think if Bug 879297 is fixed, even when timeout happens on video track, the video track continue to playback. OmxDecoder already ignores timeout error as no error.
Comment 59•11 years ago
|
||
I confirmed on v1.1 unagi that even when timeout happened on video OMXCodec, video read continued on the track.
Assignee | ||
Comment 60•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/45361ebec718
https://hg.mozilla.org/integration/mozilla-inbound/rev/86a166bf1ba7
(In reply to Sotaro Ikeda [:sotaro] from comment #59)
> I confirmed on v1.1 unagi that even when timeout happened on video OMXCodec,
> video read continued on the track.
OK. We still need to do something though since currently we're passing garbage to VideoData::Create. I'll work on it.
Whiteboard: [tef-triage][YouTubeCertBlocker+] → [tef-triage][YouTubeCertBlocker+][leave open]
Comment 61•11 years ago
|
||
Assignee | ||
Comment 62•11 years ago
|
||
It's a bad idea to have AudioFrame zero-initialize its members but VideoFrame not do it.
Attachment #758558 -
Flags: review?(chris.double)
Assignee | ||
Comment 63•11 years ago
|
||
diff -w version for clarity.
Attachment #758561 -
Flags: review?(sotaro.ikeda.g)
Assignee | ||
Comment 64•11 years ago
|
||
With this patch (and bug 879297 hacked around by disabling HWC) the HD version of the video from comment #28 works great. I can seek all over the place, and even if we block reading data, we retry after the codec times out, and it works well.
Unfortunately, the non-HD version still breaks when I seek. The weird thing is that from my logs, it seems that after we request the seek in OMXDecoder, libstagefright doesn't do any readAt calls, it just gets stuck in a never-ending series of timeouts and retries. Since it's not doing any readAts, it seems that the media cache can't be the problem. So this must be some other bug.
Comment 65•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #64)
>
> Unfortunately, the non-HD version still breaks when I seek. The weird thing
> is that from my logs, it seems that after we request the seek in OMXDecoder,
> libstagefright doesn't do any readAt calls, it just gets stuck in a
> never-ending series of timeouts and retries. Since it's not doing any
> readAts, it seems that the media cache can't be the problem. So this must be
> some other bug.
Yeah, maybe qcom's modification to OMXCodec affect to this.
Assignee | ||
Comment 66•11 years ago
|
||
I'm not sure how to debug that problem, since it seems to be happening without entering code we control, possibly even in Qualcomm's driver for which we don't have the source.
Assignee | ||
Comment 67•11 years ago
|
||
I think if I get review here I'll land the patches and close this bug and open a new one for that seeking issue.
Comment 68•11 years ago
|
||
Comment on attachment 758558 [details] [diff] [review]
Part 2: Initialize VideoFrame members to 0
Can you raise a bug to have the equivalent change done in the android code please.
Attachment #758558 -
Flags: review?(chris.double) → review+
Comment 69•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #63)
> Created attachment 758561 [details] [diff] [review]
> Part 3: Retry libstagefright audio/video decoding if it fails due to a
> timeout
Looks basically good. I have one question. Timeout could happen not only by data source, but also by content. In this case, OmxDecoder::ReadVideo() and OmxDecoder::ReadAudio() fall int almost infinite loop.
In this situation, how could nsBuiltinDecoderStateMachine shut down decoding thread?
Assignee | ||
Comment 70•11 years ago
|
||
Part 1 landed on mozilla-b2g18:
https://hg.mozilla.org/releases/mozilla-b2g18/rev/9778ac50b70b
Assignee | ||
Comment 71•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #69)
> Looks basically good. I have one question. Timeout could happen not only by
> data source, but also by content. In this case, OmxDecoder::ReadVideo() and
> OmxDecoder::ReadAudio() fall int almost infinite loop.
>
> In this situation, how could nsBuiltinDecoderStateMachine shut down decoding
> thread?
Ah, that's a very good point. I need to do something about that.
Assignee | ||
Comment 72•11 years ago
|
||
Alright, I think this is a lot better. This removes the retry loop from ReadVideo/ReadAudio; the regular video decode loop does the retries implicitly. That way we can shut down cleanly without any extra work.
Attachment #758897 -
Flags: review?(sotaro.ikeda.g)
Comment 73•11 years ago
|
||
Comment on attachment 758897 [details] [diff] [review]
Part 3 v2
Looks good to me!
Attachment #758897 -
Flags: review?(sotaro.ikeda.g) → review+
Comment 74•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #64)
>
> Unfortunately, the non-HD version still breaks when I seek. The weird thing
> is that from my logs, it seems that after we request the seek in OMXDecoder,
> libstagefright doesn't do any readAt calls, it just gets stuck in a
> never-ending series of timeouts and retries. Since it's not doing any
> readAts, it seems that the media cache can't be the problem. So this must be
> some other bug.
To debug around seek timeout, it is necessary to remember about difference between ics_chocolate(unagi, inari) and ics_strawberry(hamachi, buri). See Bug 878981.
Comment 75•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #70)
> Part 1 landed on mozilla-b2g18:
> https://hg.mozilla.org/releases/mozilla-b2g18/rev/9778ac50b70b
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/9778ac50b70b
Assignee | ||
Comment 76•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #74)
> To debug around seek timeout, it is necessary to remember about difference
> between ics_chocolate(unagi, inari) and ics_strawberry(hamachi, buri). See
> Bug 878981.
Thanks for the reminder. Unfortunately we don't have any ics_strawberry devices around here.
Assignee | ||
Comment 77•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #758561 -
Attachment is obsolete: true
Attachment #758561 -
Flags: review?(sotaro.ikeda.g)
Comment 78•11 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/6a5f8ebc836a - b2g arm opt (the emulator build) says
https://tbpl.mozilla.org/php/getParsedLog.php?id=23845842&tree=Mozilla-Inbound
SVGAnimatedPathSegList.cpp
In file included from /builds/slave/m-in-ics_a7_g-0000000000000000/build/content/media/omx/OmxDecoder.h:12,
from /builds/slave/m-in-ics_a7_g-0000000000000000/build/content/media/omx/MediaOmxReader.cpp:16:
/builds/slave/m-in-ics_a7_g-0000000000000000/build/content/media/omx/MPAPI.h: In constructor 'MPAPI::VideoFrame::VideoFrame()':
/builds/slave/m-in-ics_a7_g-0000000000000000/build/content/media/omx/MPAPI.h:48: error: class 'MPAPI::VideoFrame' does not have any field named 'mEndTimeUs'
make[7]: *** [MediaOmxReader.o] Error 1
Comment 79•11 years ago
|
||
Is it possible to have this fix in v1.0.1?
Assignee | ||
Comment 80•11 years ago
|
||
(In reply to khu from comment #79)
> Is it possible to have this fix in v1.0.1?
We could, but hasn't that ship already sailed?
Comment 81•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #77)
> https://hg.mozilla.org/releases/mozilla-b2g18/rev/50fbee667acb
Interesting, apparently our commit hooks on b2g18 don't care if there's a bug # or not in the commit message.
Comment 82•11 years ago
|
||
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #80)
> (In reply to khu from comment #79)
> > Is it possible to have this fix in v1.0.1?
>
> We could, but hasn't that ship already sailed?
But, the carrier would like to have the solution provided via FOTA as soon as possible. It means, we still need to have this solution in v1.0.1.
Do we know how long we need to put it into v1.0.1? Thanks!
Reporter | ||
Comment 83•11 years ago
|
||
(In reply to khu from comment #82)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #80)
> > (In reply to khu from comment #79)
> > > Is it possible to have this fix in v1.0.1?
> >
> > We could, but hasn't that ship already sailed?
>
> But, the carrier would like to have the solution provided via FOTA as soon
> as possible. It means, we still need to have this solution in v1.0.1.
>
> Do we know how long we need to put it into v1.0.1? Thanks!
comment 27 already confirmed that this was out for 1.01 per a discussion over the plan for YouTube. I don't think it's wise at all to introduce any distractions involving 1.01 right now given tight schedules and focus needed on 1.1 for YouTube.
Comment 84•11 years ago
|
||
Assignee | ||
Comment 85•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdb9603a0d0b
https://hg.mozilla.org/integration/mozilla-inbound/rev/976829350bd6
Flags: needinfo?(jmuizelaar)
Assignee | ||
Comment 86•11 years ago
|
||
I'm going to let this bug close. We'll open new ones for similar issues that are still around.
Whiteboard: [tef-triage][YouTubeCertBlocker+][leave open] → [tef-triage][YouTubeCertBlocker+]
Updated•11 years ago
|
Comment 87•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cdb9603a0d0b
https://hg.mozilla.org/mozilla-central/rev/976829350bd6
https://hg.mozilla.org/mozilla-central/rev/744b50c28290
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 88•11 years ago
|
||
The YouTube video experience with these patches is definitely better on b2g18. I am seeing the behavior being seen in bug 880601, however.
Keywords: verifyme
Reporter | ||
Updated•11 years ago
|
Flags: in-moztrap?
Comment 89•11 years ago
|
||
Created two cases covering under 10 minutes videos and above 10 minute videos, making sure that they are playable without issues.
https://moztrap.mozilla.org/manage/cases/?filter-id=9395
https://moztrap.mozilla.org/manage/cases/?filter-id=9396
Flags: in-moztrap?(dwatson) → in-moztrap+
You need to log in
before you can comment on or make changes to this bug.
Description
•