Closed Bug 793229 Opened 7 years ago Closed 7 years ago

Some build configs broken by shadowed constant in OmxPlugin.cpp

Categories

(Core :: Audio/Video, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: cjones, Assigned: cjones)

References

Details

Attachments

(1 file)

Our friends reported this.  I guess they build with more anal -W than we do :).
Does this do the trick?
Assignee: nobody → jones.chris.g
Attachment #663464 - Flags: feedback?(dwilson)
cjones, what build config are they using?

This change is related to bug 792988, a fix for my Otoro build break. OmxPlugin.cpp's OmxDecoder::ToVideoFrame() still has a local constant that is shadowing the name OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka. AFAIK, the Otoro build fix only needs the local constant OMX_QCOM_COLOR_FormatYVU420SemiPlanar.
Blocks: 792988
(In reply to Chris Peterson (:cpeterson) from comment #2)
> cjones, what build config are they using?
> 

No clue.

> This change is related to bug 792988, a fix for my Otoro build break.
> OmxPlugin.cpp's OmxDecoder::ToVideoFrame() still has a local constant that
> is shadowing the name OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka.
> AFAIK, the Otoro build fix only needs the local constant
> OMX_QCOM_COLOR_FormatYVU420SemiPlanar.

Yeah, my patch didn't deal with your change correctly.
Comment on attachment 663464 [details] [diff] [review]
Remove shadowed constant

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

::: media/omx-plugin/OmxPlugin.cpp
@@ -24,5 @@
>  
>  using namespace MPAPI;
>  
> -const int OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka = 0x7FA30C01;
> -

Yep, this does the trick
Attachment #663464 - Flags: feedback?(dwilson) → review?(cpeterson)
(In reply to Diego Wilson from comment #4)
> Comment on attachment 663464 [details] [diff] [review]
> Remove shadowed constant
> 
> Review of attachment 663464 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: media/omx-plugin/OmxPlugin.cpp
> @@ -24,5 @@
> >  
> >  using namespace MPAPI;
> >  
> > -const int OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka = 0x7FA30C01;
> > -
> 
> Yep, this does the trick

Diego, what is your build config (platform, toolchain)? I'd like to avoid breaking other people's code in the future, but our Tinderbox builds only test a few configurations.
Comment on attachment 663464 [details] [diff] [review]
Remove shadowed constant

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

LGTM.

I will post a separate patch later to see if the local definition of OMX_QCOM_COLOR_FormatYVU420PackedSemiPlanar32m4ka inside OmxDecoder::ToVideoFrame() is still necessary.
Attachment #663464 - Flags: review?(cpeterson) → review+
https://hg.mozilla.org/mozilla-central/rev/e96de9481915
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
I'm using ARM EABI GCC 4.4.3 on an Ubuntu 10.04 box
You need to log in before you can comment on or make changes to this bug.