Closed Bug 831747 Opened 7 years ago Closed 6 years ago

Handle video hw codec contention between App and web content in Browser app

Categories

(Firefox OS Graveyard :: General, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, b2g18+ affected)

RESOLVED DUPLICATE of bug 871485
1.1 QE2 (6jun)
blocking-b2g leo+
Tracking Status
b2g18 + affected

People

(Reporter: sotaro, Assigned: sotaro)

References

()

Details

(Whiteboard: c=video , MiniWW)

Attachments

(2 files, 1 obsolete file)

This bus is created as part of bug 830995.

unagi device fails to take photos. If OMXCodec instance is present on the device. OMXCodec instance should not be present, when camera app takes a photo on unagi device. There are two ways to solve the problem.
[1] all applications frees hw codec related resources, when it goes to background.
[2] gecko frees hw codec related resources, when document goes to background.

This bug handles [2].
It seems it is better to use AudioChannel for the problem.
Pulling in STR from bug 830995:

1. open camera app
2. switch to video mode
3. press start recording and record a ~10s video
4. press stop recording
5. press start recording and record a ~10s video
6. press stop recording
7. switch to picture mode
8. press the shutter button

Preview is now frozen; the UI is still responsive, however, and force-closing the camera app will clear the problem.

As with the log attached to comment 16, the camera driver is waiting for a signal on mJpegThreadWait: "V( 1288:0x54a) runSnapshotThread: waiting for jpeg callback."
[More info from bug 830995]

When this happens, the logcat shows:

steps 3 and 4:
E( 1712:0x6dc) ***YAMATO Enabled***
E( 1712:0x6d8) Flush VDL_stats_q: stats_ptr 0x43e404a0
E( 1712:0x6d8) Warning - previous ts > current ts. And both are non B-frames

steps 5 and 6:
E( 1712:0x6ec) ***YAMATO Enabled***
E( 1712:0x6b0) [JavaScript Error: "NS_ERROR_NOT_AVAILABLE: Component is not available" {file: "app://camera.gaiamobile.org/js/filmstrip.js" line: 489}]

The current hypothesis is that the JS error in preventing a shared resource that is used for both video decoding and JPEG encoding from being released, triggering the endless blocking on mJpegThreadWait.
[Comments from djf from bug 830995]

I haven't see that error before. If the line number is right, its occuring when we try to copy an image into a <canvas> element.  So an error from graphics somehow? I know nothing about that... out of memory? Would an image copy like that get accelerated in the graphics hardware? 

I seem to be able to reproduce this bug by launching video, playing and pausing a video and then going to the camera.  First time I take a picture, it freezes.

Also note that that call to createThumbnailFromElement() is coming from line 331.  When we get a new jpeg file from the camera, we parse its metadata, use blob.slice() to get the preview image out of it, and then create a smaller square thumbnail from that rectangular preview.

Is it possible that the camera is giving us the jpeg image before it is done writing the preview image metadata?  Maybe there is a timing issue here and if videos have been playing or recording, something is slower and we try to use the preview image before it is written to the file?  Its a complete guess, of course, but I think we had a similar issue with the video... we finished recording, but the rotation metadata wasn't fully written into the video file yet.

> To solve the problem, apps need to free OMXCodec and camera resource. Off
> the top of my head, threre are two ways to do this.
> [1] application frees hw codec related resources, when it goes to background.
> [2] gecko frees hw codec related resources, when it goes to background.

I think mike is right that the mystery exception in drawImage() is causing the camera code (in filmstrip.js) to fail to release the video. We have to fix that in gaia since it isn't a visibility issue.

Working around this issue for the Video app (and the Gallery app, too?) is more challenging to do in Gaia: it would involve remembering the current playback position and releasing the video when we're hidden, and then restarting the video and seeking to that position when we're shown.  

That doesn't seem like something we can fix in the 1.0.0 timeline in Gaia.

I'm going to file a separate bug for the Gaia fix to the camera app.  And leave this bug open for a Gecko fix to release the codecs when an app goes into the background.
Created bug 831821 for the camera exception case.
Assignee: nobody → sotaro.ikeda.g
(In reply to Mike Habicher [:mikeh] from comment #2)
> Pulling in STR from bug 830995:
> 
> 1. open camera app
> 2. switch to video mode
> 3. press start recording and record a ~10s video
> 4. press stop recording
> 5. press start recording and record a ~10s video
> 6. press stop recording
> 7. switch to picture mode
> 8. press the shutter button
> 
none of us can reproduce anymore. Then I am going to address following STR from bug 830995:

1. launching video
2. playing and pausing a video
3. going to the camera.
4. First time I take a picture, after that preview freezes.
(In reply to David Flanagan [:djf] from comment #5)
> 
> > To solve the problem, apps need to free OMXCodec and camera resource. Off
> > the top of my head, threre are two ways to do this.
> > [1] application frees hw codec related resources, when it goes to background.
> > [2] gecko frees hw codec related resources, when it goes to background.
> 
> I think mike is right that the mystery exception in drawImage() is causing
> the camera code (in filmstrip.js) to fail to release the video. We have to
> fix that in gaia since it isn't a visibility issue.
> 
> Working around this issue for the Video app (and the Gallery app, too?) is
> more challenging to do in Gaia: it would involve remembering the current
> playback position and releasing the video when we're hidden, and then
> restarting the video and seeking to that position when we're shown.  
> 
> That doesn't seem like something we can fix in the 1.0.0 timeline in Gaia.
> 
> I'm going to file a separate bug for the Gaia fix to the camera app.  And
> leave this bug open for a Gecko fix to release the codecs when an app goes
> into the background.
blocking-b2g: --- → tef?
blocking-b2g: tef? → shira?
blocking-b2g: shira? → leo?
This is wip patch. It frees video decoder when a document is hide.
It is necessary to confirm how video decoder should be freed. The patch shows just one easy way.
The user impact of this bug is not clear. If you think it should block the 1.1 release, please explain why and re-nominate.
blocking-b2g: leo? → -
created to help understanding of AudioChannelService class.
created to help understanding of nsMediaOmxDecoder class
sotaro, your object diagrams would be useful to others; you should collect all of them in a central location somewhere.
(In reply to Mike Habicher [:mikeh] from comment #13)
> sotaro, your object diagrams would be useful to others; you should collect
> all of them in a central location somewhere.

Thank you, I am going to disclose all diagrams in GitHub.
Depends on: 803471
Blocks: 803471
No longer depends on: 803471
Freeing a video decoder when the document is in the background will cause issues when it is brought back into the foreground. It won't continue playing from where it left off for example. It also prevents playing files (eg. music, podcasts, etc) in background tabs while browsing other sites.
(In reply to Chris Double (:doublec) from comment #15)
> Freeing a video decoder when the document is in the background will cause
> issues when it is brought back into the foreground. It won't continue
> playing from where it left off for example. It also prevents playing files
> (eg. music, podcasts, etc) in background tabs while browsing other sites.

Yeah, it is a problem need to prevent. But when h.264 video is played in a web page in browser in FirefoxOS, the web page continues to grab hw codec. We need a way forcibly free hw codec from the web page. I am going to limit it only to hw codec case.
(In reply to Sotaro Ikeda [:sotaro] from comment #7)
> (In reply to Mike Habicher [:mikeh] from comment #2)
> > Pulling in STR from bug 830995:
> > 
> > 1. open camera app
> > 2. switch to video mode
> > 3. press start recording and record a ~10s video
> > 4. press stop recording
> > 5. press start recording and record a ~10s video
> > 6. press stop recording
> > 7. switch to picture mode
> > 8. press the shutter button
> > 
> none of us can reproduce anymore. Then I am going to address following STR
> from bug 830995:
> 
> 1. launching video
> 2. playing and pausing a video
> 3. going to the camera.
> 4. First time I take a picture, after that preview freezes.

The above problem is addressed by changes of gaia side in Bug 837236.

But by 803471, a normal web page becomes to use hw codec, hw resource contention happens between web page and gaia app. It needs to be addressed.
blocking-b2g: - → leo?
blocking-b2g: leo? → leo+
Duplicate of this bug: 860595
set to tef? because of effecting (tef+) bug 860188 power off animation for customization
blocking-b2g: leo+ → tef?
Sotaro,

I think Bug 860188 is a special case of this bug. And if we only release hardware when web browser hide, we will still lock the hardware in that case.

Can we just release hardware when the media playback stopped (not pause case)?
(In reply to Fred Lin [:gasolin] from comment #19)
> set to tef? because of effecting (tef+) bug 860188 power off animation for
> customization

Browser can not use hw codec in v1.01. It is possible from v1.1. So it is not tef+ but leo+.
(In reply to Chiajung Hung [:chiajung] from comment #20)
> Sotaro,
> 
> I think Bug 860188 is a special case of this bug. And if we only release
> hardware when web browser hide, we will still lock the hardware in that case.
> 
> Can we just release hardware when the media playback stopped (not pause
> case)?

I generalize this bug's title to handle hw video codec usage contention between app and web content.
Summary: Free video codec of video tag, when a document is hidden → Handle video hw codec contention between App and web content in Browser app
(In reply to Fred Lin [:gasolin] from comment #19)
> set to tef? because of effecting (tef+) bug 860188 power off animation for
> customization

bug 860188 is not a tef+ bug. Is there a other bug related to this bug?
sorry for miss bug number,

tef+ bug 860595 : power off animation for customization

, not bug 860188
elaboration for bug 860188:

when use power on video with mp4 (which works fine),
start the video app will print some OMXCodec error and can't show any video.
then power off can't decode mp4 because of the OMXCodec error.
ooh, when I say bug 860188, I mean bug 860595 ...
(In reply to Fred Lin [:gasolin] from comment #26)
> ooh, when I say bug 860188, I mean bug 860595 ...

I checked bug 860595, it should not be handled as dupe of this bug. It should be handled by power on/off animation.
(In reply to Sotaro Ikeda [:sotaro] from comment #27)
> (In reply to Fred Lin [:gasolin] from comment #26)
> > ooh, when I say bug 860188, I mean bug 860595 ...
> 
> I checked bug 860595, it should not be handled as dupe of this bug. It
> should be handled by power on/off animation.

change to leo? from the above comment.
blocking-b2g: tef? → leo?
s(In reply to Sotaro Ikeda [:sotaro] from comment #27)
> (In reply to Fred Lin [:gasolin] from comment #26)
> > ooh, when I say bug 860188, I mean bug 860595 ...
> 
> I checked bug 860595, it should not be handled as dupe of this bug. It
> should be handled by power on/off animation.

Same thing is already done by Camera/Gallery/Video app. For v1.01, hw codec is handled by apps. It is too high risk to add this mechanism for v1.01.
Duplicate of this bug: 860188
What exactly is this bug trying to fix at this point? What's the user impact?
(In reply to Alex Keybl [:akeybl] from comment #31)
> What exactly is this bug trying to fix at this point? What's the user impact?

Detatch hw video code from a web content in Browser app, when an app try to use hw video codec. It Firefox OS do not have this capability. A web content could block camera/video app working.
Putting it back to leo+ as it was 2 weeks ago, it seems clear now this is not a tef? bug and the flag should not have much more churn.
blocking-b2g: leo? → leo+
tracking-b2g18: --- → +
Flags: in-moztrap?
Status: NEW → ASSIGNED
Whiteboard: c=performance
Whiteboard: c=performance → c=
Target Milestone: --- → 1.1 QE2
Priority: -- → P1
Whiteboard: c= → c=video
Comment on attachment 707269 [details] [diff] [review]
wip patch: free video Decoder when document is hide

The patch can not solve all cases in v1.1. So obsolete it.
Attachment #707269 - Attachment is obsolete: true
Depends on: 871485
Whiteboard: c=video → c=video , MiniWW
current patches in Bug 871485 fix this problem. I confirmed on v1.1 buri.
resolved as dup of bug 871485 per comment 35.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 871485
Flags: in-moztrap?
You need to log in before you can comment on or make changes to this bug.