Status

()

defect
P2
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: ayang, Assigned: ayang)

Tracking

unspecified
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

(Whiteboard: [2016-GBT-Y])

Attachments

(1 attachment, 5 obsolete attachments)

Blocks: 1233410
Summary: Support sw codec on ics for emulator try test. → Support sw codec
See Also: → 1231690
Summary: Support sw codec → Support sw video codec
Posted patch refactory_omx_output (obsolete) — Splinter Review
Assignee: nobody → ayang
Depends on: 1224889
Posted patch support_sw_video_codec (obsolete) — Splinter Review
1. fix input buffer size problem.
2. add SW codec output colour format.
3. refactory audio/video output to MediaDataHelper.
Attachment #8700993 - Attachment is obsolete: true
Attachment #8700994 - Attachment is obsolete: true
Attachment #8701738 - Flags: review?(sotaro.ikeda.g)
Attachment #8701738 - Flags: review?(sotaro.ikeda.g)
Priority: -- → P2
Posted patch support_sw_video_codec (obsolete) — Splinter Review
Attachment #8701738 - Attachment is obsolete: true
Posted patch support_sw_video_codec (obsolete) — Splinter Review
Attachment #8704515 - Attachment is obsolete: true
Attachment #8705642 - Flags: review?(sotaro.ikeda.g)
Comment on attachment 8705642 [details] [diff] [review]
support_sw_video_codec

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

::: dom/media/platforms/omx/GonkOmxPlatformLayer.cpp
@@ +535,5 @@
> +  // H264 and H263 has different profiles, software codec doesn't support high profile.
> +  // So we use hardware codec only.
> +  if (!IsInEmulator() &&
> +      (mInfo->mMimeType.EqualsLiteral("video/avc") ||
> +       mInfo->mMimeType.EqualsLiteral("video/3gp"))) {

Don't we need to check mime types like in GonkDecoderModule::SupportsMimeType(), do we?
  https://dxr.mozilla.org/mozilla-central/source/dom/media/platforms/gonk/GonkDecoderModule.cpp#64
Attachment #8705642 - Flags: review?(sotaro.ikeda.g)
Comment on attachment 8705642 [details] [diff] [review]
support_sw_video_codec

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

::: dom/media/platforms/omx/OmxDataDecoder.cpp
@@ +1020,5 @@
> +      // Get YUV VideoData, it uses more CPU, in most cases, on software codec.
> +      data = CreateYUV420VideoData(aBufferData);
> +    }
> +
> +    // Update vodeo time code, duration... from the raw data.

nit: video
Comment on attachment 8705642 [details] [diff] [review]
support_sw_video_codec

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

::: dom/media/platforms/omx/GonkOmxPlatformLayer.cpp
@@ +535,5 @@
> +  // H264 and H263 has different profiles, software codec doesn't support high profile.
> +  // So we use hardware codec only.
> +  if (!IsInEmulator() &&
> +      (mInfo->mMimeType.EqualsLiteral("video/avc") ||
> +       mInfo->mMimeType.EqualsLiteral("video/3gp"))) {

From the comment, it was not clear why video/mp4 video/mp4v-es are not handled here. Can you make it clear?
(In reply to Sotaro Ikeda [:sotaro] from comment #10)
> Comment on attachment 8705642 [details] [diff] [review]
> support_sw_video_codec
> 
> Review of attachment 8705642 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/platforms/omx/GonkOmxPlatformLayer.cpp
> @@ +535,5 @@
> > +  // H264 and H263 has different profiles, software codec doesn't support high profile.
> > +  // So we use hardware codec only.
> > +  if (!IsInEmulator() &&
> > +      (mInfo->mMimeType.EqualsLiteral("video/avc") ||
> > +       mInfo->mMimeType.EqualsLiteral("video/3gp"))) {
> 
> From the comment, it was not clear why video/mp4 video/mp4v-es are not
> handled here. Can you make it clear?

I forgot mp4. :)
Added.
Attachment #8705642 - Attachment is obsolete: true
Attachment #8707727 - Flags: review?(sotaro.ikeda.g)
Attachment #8707727 - Flags: review?(sotaro.ikeda.g) → review+
Thanks for review!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4be86232c89c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.