Closed Bug 969312 Opened 11 years ago Closed 6 years ago

[MediaEncoder] Support data from GonkCameraSource in HW video recording path.

Categories

(Core :: Audio/Video: Recording, defect, P3)

ARM
Gonk (Firefox OS)
defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: jhlin, Unassigned)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files, 2 obsolete files)

Android video recorder and the recorder of mozCamera use a "metadata" format that both HW encoder and camera understand. In their implementation, the effort of buffer copying is very small (memcpy() 8 bytes on Nexus 4). In current Gecko GetUserMedia() implementation on B2G, the camera input data is from preview window. Since HW encoder doesn't necessary support its pixel format, full frame color conversion/copying is needed. For B2G HW video encoding, getting camera input data from GonkCameraSource seems a better choice.
This is for video recording performance improvement. 1. Connect camera recording callback for media recording flow 2. Keep camera preview callback for media playback(<video> tag) We need to be able to carry different video chunk in media track for different track listener, media recorder and <video> I think this one should be a blocker for 1.5
Blocks: MediaEncoder
(In reply to C.J. Ku[:CJKu] from comment #1) > This is for video recording performance improvement. > 1. Connect camera recording callback for media recording flow > 2. Keep camera preview callback for media playback(<video> tag) > We need to be able to carry different video chunk in media track for > different track listener, media recorder and <video> > > I think this one should be a blocker for 1.5 Current idea is: 1. Refactoring - move most of B2G camera related code out from MediaEngineWebRTC/MediaEngineWebRTCVideoSource into its own file 2. Add new data source (GonkCameraSource) 3. Create new chunk data type for GonkCameraSource (new gfx::layers::Image subclass) 4. Make MediaRecorder/MediaEncoder aware of and receive data from new source 5. Let OMXVideoEncoder accept new data type in 3, and still handle muted/black screen data correctly Looks like 1~3 can be tracked with Bug 938034, so change the summary of this bug to address 4 & 5.
Depends on: 938034
Summary: [MediaEncoder] On B2G, get camera data from GonkCameraSource for HW video encoder. → [MediaEncoder] Support data from GonkCameraSource in HW video recording path.
Component: Video/Audio → Video/Audio: Recording
Make MediaEncoder a direct listener of source media stream, and accepts video data only from direct listener callback.
Attachment #8381190 - Flags: feedback?(rlin)
Attachment #8381190 - Flags: feedback?(cku)
Enable 'metadata in buffer' mode to eliminate full size buffer copying.
Attachment #8381191 - Flags: feedback?(rlin)
Attachment #8381191 - Flags: feedback?(cku)
Still missing: 1. Notification of recording start/stop. 2. Black metadata buffer for muted frame.
Comment on attachment 8381191 [details] [diff] [review] Part 2: [WIP] Accept video data from GonkCameraSource in OMXVideoEncoder. Review of attachment 8381191 [details] [diff] [review]: ----------------------------------------------------------------- add peter
Attachment #8381191 - Flags: feedback?(pchang)
Comment on attachment 8381190 [details] [diff] [review] Part 1: [WIP] Make MediaEncoder aware of data from GonkCameraSource. Review of attachment 8381190 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/encoder/MediaEncoder.h @@ +84,5 @@ > > /** > + * Notified when AppendToTrack() of source media stream is called. It's used > + * to receive data from Camera. > + */ #ifdef MOZ_B2G_CAMERA
Attachment #8381190 - Flags: feedback?(cku) → feedback+
Comment on attachment 8381191 [details] [diff] [review] Part 2: [WIP] Accept video data from GonkCameraSource in OMXVideoEncoder. Review of attachment 8381191 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/omx/OMXCodecWrapper.cpp @@ +263,4 @@ > if (!aImage) { > +#ifdef MOZ_B2G_CAMERA > + inBuf->setRange(0, 0); > + dstSize = 0; Is this code design for blank image while mediarecoder::pause? @@ +337,5 @@ > + IntSize imgSize = img->GetSize(); > + CODEC_ERROR("metadata buffer:%p data:%p len:%d time:%lld size:%dx%d flags:%x", > + buffer, src, len, aTimestamp, imgSize.width, imgSize.height, aInputFlags); > + inBuf->setRange(0, len); > + memcpy(dst, src, len); Only META data copy? there should be no image copy here
Attachment #8381191 - Flags: feedback?(cku) → feedback+
Attachment #8381191 - Flags: feedback?(rlin) → feedback+
Comment on attachment 8381190 [details] [diff] [review] Part 1: [WIP] Make MediaEncoder aware of data from GonkCameraSource. Review of attachment 8381190 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/encoder/MediaEncoder.h @@ +85,5 @@ > /** > + * Notified when AppendToTrack() of source media stream is called. It's used > + * to receive data from Camera. > + */ > + virtual void NotifyRealtimeData(MediaStreamGraph* aGraph, TrackID aID, Also need flag add on notifier(MSG).
Attachment #8381190 - Flags: feedback?(rlin) → feedback+
Comment on attachment 8381191 [details] [diff] [review] Part 2: [WIP] Accept video data from GonkCameraSource in OMXVideoEncoder. Review of attachment 8381191 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/omx/OMXCodecWrapper.cpp @@ +263,4 @@ > if (!aImage) { > +#ifdef MOZ_B2G_CAMERA > + inBuf->setRange(0, 0); > + dstSize = 0; Suggest to add TODO here for blank image insertion for b2g @@ +282,5 @@ > MOZ_ASSERT(aWidth == img->GetSize().width && > aHeight == img->GetSize().height); > > + if (format != ImageFormat::B2G_METADATA_BUFFER_CAMERA) { > + inBuf->setRange(0, yLen + uvLen); I would prefer to move setRange to GRALLOC_PLANAR_YCBCR and PLANAR_YCBCR @@ +326,5 @@ > dst); > +#ifdef MOZ_B2G_CAMERA > + } else if (format == ImageFormat::B2G_METADATA_BUFFER_CAMERA) { > + MediaBuffer* buffer = static_cast<MediaBuffer*>(img->GetImplData()); > + if (img->IsSentToCompositor() || !buffer) { I saw img->MarkSent() called in line 345, you add isSentToCompositor() call here. Does it mean it is possible to encode the same image twice? If so, why do we have same image for encoding twice? @@ +337,5 @@ > + IntSize imgSize = img->GetSize(); > + CODEC_ERROR("metadata buffer:%p data:%p len:%d time:%lld size:%dx%d flags:%x", > + buffer, src, len, aTimestamp, imgSize.width, imgSize.height, aInputFlags); > + inBuf->setRange(0, len); > + memcpy(dst, src, len); Please add comment here to address only meta data(buffer) copy here.
Attachment #8381191 - Flags: feedback?(pchang) → feedback+
Blocks: 984239
Not a blocker for H.264 hardware support for webRTC in v1.5
Blocks: b2g-multimedia
No longer blocks: 984239
Per-talk with John Lin, steal this bug from him.
Assignee: jolin → ctai
Rebased and carry cjku and rlin's f+.
Attachment #8381190 - Attachment is obsolete: true
Attachment #8453641 - Flags: feedback+
Rebased and carry cjku and rlin's f+.
Attachment #8381191 - Attachment is obsolete: true
Attachment #8453642 - Flags: feedback+
Blocks: 1074675
Assignee: ctai → ayang
Alfredo - Are you planning on finishing these patches? You might also look at the changes to the getUserMedia capture code expected in the webrtc.org 43 update, which is about to land. See bug 1198458; there's a patch queue you can apply to see the updates. There's code to support texture backing of captured data, I believe. Thanks!
Rank: 28
Flags: needinfo?(ayang)
Priority: -- → P2
(In reply to Maire Reavy [:mreavy] (Back Dec 21st - ni?:mreavy) from comment #15) > Alfredo - Are you planning on finishing these patches? > > You might also look at the changes to the getUserMedia capture code expected > in the webrtc.org 43 update, which is about to land. See bug 1198458; > there's a patch queue you can apply to see the updates. There's code to > support texture backing of captured data, I believe. > Thanks! Thanks for info! I'm no longer working on it. Munro may have interest to complete it.
Assignee: ayang → nobody
Flags: needinfo?(ayang)
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3
Closing as we are not working on Firefox OS anymore.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: