Closed
Bug 925381
Opened 11 years ago
Closed 11 years ago
Fail to seek mpeg4 files in video player
Categories
(Firefox OS Graveyard :: General, defect, P1)
Tracking
(blocking-b2g:koi+, firefox25 wontfix, firefox26 fixed, firefox27 fixed, b2g-v1.2 fixed)
People
(Reporter: bhargavg1, Assigned: sotaro)
Details
(Keywords: regression, Whiteboard: [PRISM:552721])
Attachments
(2 files, 4 obsolete files)
5.32 MB,
video/3gpp
|
Details | |
13.74 KB,
patch
|
sotaro
:
review+
|
Details | Diff | Splinter Review |
On playing mpeg4 files in video app, seeking fwd by scrubbing the bar doesn't play the video only audio is heard
Assignee | ||
Comment 1•11 years ago
|
||
Can you provide hardware info and software version?
Assignee | ||
Comment 3•11 years ago
|
||
(In reply to bhargavg1 from comment #2) > Hardware: MSM7x27a QRD device > BuildId: 20131009193215 can you provide git info of gecko and gaia? The hardware seems like not Firefox OS phone. Does the soft uses caf's android ICS code? And can you provide a content that generate the problem?
Flags: needinfo?(bhargavg1)
(In reply to Sotaro Ikeda [:sotaro] from comment #3) > (In reply to bhargavg1 from comment #2) > > Hardware: MSM7x27a QRD device > > BuildId: 20131009193215 > > can you provide git info of gecko and gaia? The hardware seems like not > Firefox OS phone. Does the soft uses caf's android ICS code? And can you > provide a content that generate the problem? SHA1's Gecko: c038ee40141d22e088e4777355d2be0c3e673aed Gaia: 672c47bf94b69a329e0aacb9228a6aa16ade6226
Flags: needinfo?(bhargavg1)
CAF manifest file @https://www.codeaurora.org/cgit/quic/lf/manifest/tree/caf_AU_LINUX_GECKO_B2G_ICS_1.2.01.02.00.019.071.xml?id=refs/heads/release
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → sotaro.ikeda.g
Assignee | ||
Comment 7•11 years ago
|
||
Thanks for the infos and video. Confirmed the problem on my master hamachi. It is a regression.
Assignee | ||
Updated•11 years ago
|
Keywords: regression,
regressionwindow-wanted
Assignee | ||
Comment 8•11 years ago
|
||
gralloc buffers seems not returned to ANativeWindow correctly during seeking.
Assignee | ||
Comment 9•11 years ago
|
||
Confirmed that the problem also happens by attachment 742194 [details] in Bug 864230.
Updated•11 years ago
|
QA Contact: nkot
Comment 10•11 years ago
|
||
I quickly tested with several .mp4 and .3gpp videos on keon with a recent-ish (~1 week old) gecko+gaia, and could not reproduce the issue. Could this be a bad interaction between how video playback work and the device running out of pmem?
Reporter | ||
Comment 11•11 years ago
|
||
the first time the issue was noticed in the following CAF manifest AU https://www.codeaurora.org/cgit/quic/lf/manifest/tree/caf_AU_LINUX_GECKO_B2G_ICS_1.2.01.02.00.019.057.xml?id=refs/heads/release Gecko: 52eeb289cc638408fff6be89835e2c1e121bd955 Gaia: b0e4a1333bb7bf0a749a384ba99e4f03f111e39a
Comment 12•11 years ago
|
||
(In reply to Nicolas Silva [:nical] from comment #10) > I quickly tested with several .mp4 and .3gpp videos on keon with a > recent-ish (~1 week old) gecko+gaia, and could not reproduce the issue. > > Could this be a bad interaction between how video playback work and the > device running out of pmem? I could reproduce it with the video attached in comment 9, I am rying to find a regression range, so far tested on the latest m-c and 09/18 m-c - the issue reproduces on both
Comment 13•11 years ago
|
||
How do we know that this is a regression issue? Is it from 1.1 or within the 1.2 cycle?
Flags: needinfo?(nkot)
Assignee | ||
Comment 14•11 years ago
|
||
It is a regression from 1.1. on v1.2, video playback does not work correctly until Bug 907745 was fixed. If the mpeg4 seek has a problem all time since Bug 907745 was fixed, there is no regression range in v1.2.
Assignee | ||
Comment 15•11 years ago
|
||
I understand the reason of the regression from v1.1. Clear 'regressionwindow-wanted'.
Keywords: regressionwindow-wanted
Assignee | ||
Comment 16•11 years ago
|
||
As in Bug 864230, mpeg4 hw codec requires more buffer than h.264. b2g v1.1 achieved the smooth mpeg4 playback. But it was not easy. During mpeg4 playback, buffers usage is very very tight. And in v1.1, composer holds 1 buffer at most. But in v1.2, composer holds 2 buffer. During seeking, it makes buffer starvation on hamachi and leo devices.
Assignee | ||
Comment 17•11 years ago
|
||
on v1.1, composer holds 1 buffer. But on v1.2, composer holds 2 buffers. I locally created a v1.2 ROM that composer does not hold a buffer during seeking. By the ROM, v1.2 hamachi succeeds mpeg4 seek. But it causes black screen during seeking. It is not acceptable from user experience point of view. To fix it, need add a way to hold only 1 buffer during seek.
Assignee | ||
Comment 18•11 years ago
|
||
android's BufferQueue already implements similar functions BufferQueue::freeAllBuffersExceptHeadLocked(). gecko also needs to implement similar thing. http://androidxref.com/4.3_r2.1/xref/frameworks/native/libs/gui/BufferQueue.cpp#988
Updated•11 years ago
|
blocking-b2g: koi? → koi+
Priority: -- → P1
Comment 19•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #14) > It is a regression from 1.1. on v1.2, video playback does not work correctly > until Bug 907745 was fixed. If the mpeg4 seek has a problem all time since > Bug 907745 was fixed, there is no regression range in v1.2. Thanks Sotaro! yes, the issue doesn't reproduce on v1.1, I've tested that too. Please let me know if you need any help determining regression within 1.2 cycle. From my testing, i could consistently reproduce this bug starting 09/13, but it's possible that it occurred earlier, it's just really hard to say due to other playback issue on earlier builds. BuildID: 20130913040201 Gaia: 8ccb741b6adcfe9a78b842c17e5874242c0f8b86 Gecko: b9029b1de410 Version: 26.0a1
Flags: needinfo?(nkot)
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to nkot from comment #19) > (In reply to Sotaro Ikeda [:sotaro] from comment #14) > yes, the issue doesn't reproduce on v1.1, I've tested that too. Please let > me know if you need any help determining regression within 1.2 cycle. From > my testing, i could consistently reproduce this bug starting 09/13, but it's > possible that it occurred earlier, On v1.2, video playback was basically broken until Bug 901224 and Bug 907745 were fixed. Because of the broken playback, seek did not cause the buffer starvation until 09/13 on v1.2, I think.
Assignee | ||
Comment 21•11 years ago
|
||
Thanks for the confirmation.
Assignee | ||
Comment 22•11 years ago
|
||
By applying the patch, mpeg4 seek works on master hamachi.
Reporter | ||
Comment 23•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #22) > Created attachment 816050 [details] [diff] [review] > patch - return all video frames except front one during seek > > By applying the patch, mpeg4 seek works on master hamachi. Can you let me know the based build id that you are using. Facing issues in applying the patch
Assignee | ||
Comment 24•11 years ago
|
||
(In reply to bhargavg1 from comment #23) > Can you let me know the based build id that you are using. Facing issues in > applying the patch I use master gecko, git commit id is following. gecko: ad67fba436dbdb4b8f66be7e9b1783bd41bdcd90
Reporter | ||
Comment 25•11 years ago
|
||
Yes works for me too !
Assignee | ||
Comment 26•11 years ago
|
||
(In reply to bhargavg1 from comment #25) > Yes works for me too ! Thanks for the confirmation.
Reporter | ||
Comment 27•11 years ago
|
||
(In reply to Sotaro Ikeda [:sotaro] from comment #26) > (In reply to bhargavg1 from comment #25) > > Yes works for me too ! > > Thanks for the confirmation. Can we merge this change?
Flags: needinfo?(sotaro.ikeda.g)
Updated•11 years ago
|
status-b2g-v1.2:
--- → affected
Updated•11 years ago
|
Target Milestone: --- → 1.2 C3(Oct25)
Assignee | ||
Comment 28•11 years ago
|
||
(In reply to bhargavg1 from comment #27) > > Thanks for the confirmation. > > Can we merge this change? Yes, I am going to update the patch.
Flags: needinfo?(sotaro.ikeda.g)
Assignee | ||
Comment 29•11 years ago
|
||
Attachment #816050 -
Attachment is obsolete: true
Assignee | ||
Comment 30•11 years ago
|
||
Attachment #817435 -
Attachment is obsolete: true
Assignee | ||
Comment 31•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #817440 -
Flags: review?(nical.bugzilla)
Assignee | ||
Updated•11 years ago
|
Attachment #817441 -
Flags: review?(chris.double)
Updated•11 years ago
|
Attachment #817441 -
Flags: review?(chris.double) → review+
Comment 32•11 years ago
|
||
Comment on attachment 817440 [details] [diff] [review] patch part 1 - Change to /gfx/layers/ Review of attachment 817440 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. I just have the following remark that we are at the same time trying to enforce constraints on the life cycles of textures (like in this bug) while at the same time trying to reduce them in order to be able to share them to several layers at the same time (Bug 912897). I don't think those two approaches will play well together in the long run. We'll figure that out later, but we should be careful when writing code that expects TextureClients to strictly belong to one CompositableClient (or ImageClient in this case), because we are trying to move away from that. ::: gfx/layers/ImageContainer.cpp @@ +190,1 @@ > // Let ImageClient to release all TextureClients. nit: you can also remove the "to" to fix this comment since you are modifying the surrounding code. @@ +202,5 @@ > + void > +ImageContainer::ClearAllImages() > +{ > + if (IsAsync()) { > + // Let ImageClient to release all TextureClients. nit: same here, let's remove the "to" ::: gfx/layers/ImageContainer.h @@ +390,5 @@ > + void ClearAllImages(); > + > + /** > + * Clear all images except current one. > + * Let ImageClient to release all TextureClients except front one. nit: trailing spaces
Attachment #817440 -
Flags: review?(nical.bugzilla) → review+
Updated•11 years ago
|
Component: Gaia::Video → General
Assignee | ||
Comment 33•11 years ago
|
||
Roll up patch. Apply comments. Carry 'nical.bugzilla: review+' and chris.double: review+.
Attachment #817440 -
Attachment is obsolete: true
Attachment #817441 -
Attachment is obsolete: true
Attachment #818029 -
Flags: review+
Assignee | ||
Comment 34•11 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=8b4008cbe96c
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 35•11 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/7b73d5834399
Keywords: checkin-needed
Comment 36•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7b73d5834399
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 37•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/5ae9a7f6cba8
Updated•11 years ago
|
Target Milestone: 1.2 C3(Oct25) → 1.3 Sprint 3 - 10/25
You need to log in
before you can comment on or make changes to this bug.
Description
•