Closed
Bug 1104663
Opened 10 years ago
Closed 9 years ago
[gonk-l] libopus get compile error with -O3
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 1056337
People
(Reporter: seinlin, Assigned: seinlin)
References
Details
Attachments
(2 files, 1 obsolete file)
1016 bytes,
patch
|
cpeterson
:
feedback+
|
Details | Diff | Splinter Review |
1.24 KB,
patch
|
rillian
:
review-
cpeterson
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Hi Chris, Could you have a look to this patch? Thanks!
Attachment #8528890 -
Flags: feedback?(cpeterson)
Comment 2•10 years ago
|
||
Comment on attachment 8528890 [details] [diff] [review] bug-1104663-libopus.patch Review of attachment 8528890 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/libopus/moz.build @@ +91,2 @@ > if CONFIG['MOZ_OPTIMIZE']: > + if CONFIG['ANDROID_VERSION'] >= '21': Is this gcc bug fixed in recent gcc versions? Can you can add a gcc version check so we don't have to deoptimize libopus forever? :) @@ +91,3 @@ > if CONFIG['MOZ_OPTIMIZE']: > + if CONFIG['ANDROID_VERSION'] >= '21': > + # -O3 hit gcc bug in gonk-l, fallback to use -Os The comment should probably include a link to a bug report of the gcc bug. @@ +95,5 @@ > + '-Os', > + ] > + CXXFLAGS += [ > + '-Os', > + ] Is the -Os workaround needed for both CFLAGS and CXXFLAGS?
Attachment #8528890 -
Flags: feedback?(cpeterson) → feedback+
Assignee | ||
Comment 3•10 years ago
|
||
Hi Chris, Could you review this patch? Thanks! I checked and verified that we do not need to add Os in this makefile as it will be passed from above level.
Attachment #8529492 -
Flags: review?(cpeterson)
Assignee | ||
Comment 4•10 years ago
|
||
Try result: https://tbpl.mozilla.org/?tree=Try&rev=cb2db18ad056
Assignee | ||
Comment 5•10 years ago
|
||
Update the comment.
Attachment #8529492 -
Attachment is obsolete: true
Attachment #8529492 -
Flags: review?(cpeterson)
Attachment #8529499 -
Flags: review?(cpeterson)
Comment 6•10 years ago
|
||
Please check that this issue isn't fixed by bug 1056337.
Depends on: 1056337
Comment 7•10 years ago
|
||
Comment on attachment 8529499 [details] [diff] [review] bug-1104663.patch Review of attachment 8529499 [details] [diff] [review]: ----------------------------------------------------------------- Looks OK to me, but I am not a reviewer for this component. Ralph? Since this is a gcc bug, it would be nice if we could check the compiler version < 4.8.2 instead of just Android API Level. (The gcc bug report suggests the bug was fixed in upstream gcc 4.8.2). When we start building with a future Android NDK's fixed gcc, then we will automatically benefit from -O3 without having to remember to revert this libopus change. :)
Attachment #8529499 -
Flags: review?(giles)
Attachment #8529499 -
Flags: review?(cpeterson)
Attachment #8529499 -
Flags: feedback+
Comment 8•10 years ago
|
||
Comment on attachment 8529499 [details] [diff] [review] bug-1104663.patch Review of attachment 8529499 [details] [diff] [review]: ----------------------------------------------------------------- Pretty much what cpeterson said. If the issue is a gcc internal compiler error, the issue is the the toolchain version, not the android version, so it would be better to check for that. Also, per comment #6, have you checked that the patch from bug 1056337 does not address this problem?
Attachment #8529499 -
Flags: review?(giles) → review-
Assignee | ||
Comment 10•9 years ago
|
||
This issue get fixed in bug 1056337.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•