Closed Bug 1038037 Opened 5 years ago Closed 5 years ago

[dolphin][flame] B2G crash when opening malformed MP4A-LATM audio streaming over RTSP

Categories

(Firefox OS Graveyard :: RTSP, defect)

x86_64
Linux
defect
Not set

Tracking

(blocking-b2g:1.4+, b2g-v1.3 wontfix, b2g-v1.3T wontfix, b2g-v1.4 fixed, b2g-v2.0 verified, b2g-v2.1 verified)

RESOLVED FIXED
2.1 S1 (1aug)
blocking-b2g 1.4+
Tracking Status
b2g-v1.3 --- wontfix
b2g-v1.3T --- wontfix
b2g-v1.4 --- fixed
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: angelc04, Assigned: ethan)

References

Details

(Keywords: crash, Whiteboard: [sprd333131][partner-blocker][b2g-crash] [p=2])

Crash Data

Attachments

(3 files, 3 obsolete files)

Steps to reproduce
---------------------------------------------------------------------------
1. Launch Browser
2. open http://116.228.149.59/streaming/audio/audio_index.html
3. Tap on any audio under "AUDIO 3GP" section
   --> b2g process restarts

Crash report: https://crash-stats.mozilla.com/report/index/28dd1d0c-6283-4f87-a82a-ee7762140714

This can be reproduced on both Flame v1.4 and dolphin.

Test build
------------------------------------------------------------------------
Gaia      b0e9b4bdb39c5eb93a6783a34624ffc84f62b126
Gecko     55a0cfe90ffaa9d5c8a0c163e8bc14e8487af413
BuildID   20140710102927
Version   30.0
ro.build.version.incremental=282
ro.build.date=Thu Jul 10 10:27:07 CST 2014
blocking-b2g: --- → 1.4?
Whiteboard: [sprd333131][partner-blocker]
The crash report looks like Bug 1035755 - [V1.4][Camera][Dolphin] Repeatedly tap video record button causes app crash
(In reply to pcheng from comment #1)
> The crash report looks like Bug 1035755 - [V1.4][Camera][Dolphin] Repeatedly
> tap video record button causes app crash

pls ignore above comment
Flame v1.3 base image also has this problem.
We need a crash report with a build that's associated with symbols.  Please try on flame 1.4 build.
Crash Signature: [@ libxul.so@0x139cf50 | __write_to_log_init ]
Flags: needinfo?(nhirata.bugzilla)
Keywords: qawanted
seems the same issue with bug: Bug 1035755 - [V1.4][Camera][Dolphin] Repeatedly tap video record button causes app crash
Whiteboard: [sprd333131][partner-blocker] → [sprd333131][partner-blocker][b2g-crash]
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1035755
(In reply to James Zhang (Spreadtrum) from comment #7)
> 
> *** This bug has been marked as a duplicate of bug 1035755 ***

james, this bug might be different reason as bug 1035755 (bcz 1035755 is a regression from Bug 1029719, and this one can be reproduced even with v1.3).

So reopen it.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
I will double check after bug 1035755 is fixed.
Flags: needinfo?(pcheng)
qawanted request has been fulfilled on comment 5.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
Flags: needinfo?(jmitchell)
Bug 1035755 landed yesterday.  This should be in today's build to test for flame.
Crash Signature: [@ libxul.so@0x139cf50 | __write_to_log_init ] → [@ libxul.so@0x139cf50 | __write_to_log_init ] [@ __libc_android_abort | __write_to_log_init ]
I tested on today's flame and can still reproduce the crash:  
https://crash-stats.mozilla.com/report/index/05269eae-3278-476b-8bca-e841d2140717

The crash stack doesn't make sense to me still.

There is a chance we might need to separate what happens on flame versus dolphin for this crash.
Flags: needinfo?(nhirata.bugzilla)
I also tested on flame and dolphin today. This crash still happens.
Flags: needinfo?(pcheng)
Sounds like a 100% repro crash - plussing.

Thomas please find someone to check on device rather than trying to wait for the right crash stack.
blocking-b2g: 1.4? → 1.4+
I am going to investigate this bug. This can be reproed on my Nexus 5 as well.
Assignee: nobody → bwu
Update the current status. 
The following is bt with GDB attached. It crashes in MakeAACCodecSpecificData()

#0  __libc_android_abort () at bionic/libc/unistd/abort.c:81
#1  0xb6f4a788 in abort () at bionic/libc/arch-arm/bionic/abort_arm.S:41
#2  0xb6f864c6 in __android_log_assert (cond=<optimized out>, tag=0xb5f1a4c4 "APacketSource", fmt=0xb5ee4d21 "%s") at system/core/liblog/logd_write.c:261
#3  0xb4e3d982 in android::MakeAACCodecSpecificData (params=<optimized out>) at ../../../../gecko/netwerk/protocol/rtsp/rtsp/APacketSource.cpp:222
#4  0xb4e3e362 in android::APacketSource::APacketSource (this=0xadf5b870, sessionDesc=..., index=1) at ../../../../gecko/netwerk/protocol/rtsp/rtsp/APacketSource.cpp:474
#5  0xb4e458dc in android::RtspConnectionHandler::setupTrack (this=0xadc84940, index=1) at ../../../../gecko/netwerk/protocol/rtsp/rtsp/RTSPConnectionHandler.h:1325
#6  0xb4e46232 in android::RtspConnectionHandler::onMessageReceived (this=0xadc84940, msg=...) at ../../../../gecko/netwerk/protocol/rtsp/rtsp/RTSPConnectionHandler.h:551
#7  0xb6c0946e in android::ALooperRoster::deliverMessage (this=0xb6c0e020, msg=...) at frameworks/av/media/libstagefright/foundation/ALooperRoster.cpp:134
#8  0xb6c08f4a in android::ALooper::loop (this=0xacf54430) at frameworks/av/media/libstagefright/foundation/ALooper.cpp:209
#9  0xb6ee7a1e in android::Thread::_threadLoop (user=0xadf54980) at frameworks/native/libs/utils/Threads.cpp:800
#10 0xb6ee7582 in thread_data_t::trampoline (t=<optimized out>) at frameworks/native/libs/utils/Threads.cpp:115
#11 0xb6f3aba4 in __thread_entry (func=0xb6ee7529 <thread_data_t::trampoline(thread_data_t const*)>, arg=0xadf4ace0, tls=0xabc8df00) at bionic/libc/bionic/pthread_create.cpp:92
#12 0xb6f3ad20 in pthread_create (thread_out=0xbeec5304, attr=<optimized out>, start_routine=0x78, arg=0xadf4ace0) at bionic/libc/bionic/pthread_create.cpp:201
#13 0x00000000 in ?? ()
Hi Ethan:
As discussed, please check the sdp packet first and the solution might be similar to Bug 1009497.
Assignee: bwu → ettseng
Thanks! Benjamin and Ethan.
(In reply to Benjamin Chen [:bechen] from comment #17)
> Hi Ethan:
> As discussed, please check the sdp packet first and the solution might be
> similar to Bug 1009497.

Okay. I will look into this bug.
Component: Gaia::Browser → Video/Audio
Product: Firefox OS → Core
Captured RTSP packets.
According to the captured RTSP packets and the backtrace in comment 16, we know the crash is due to the assertion at this line:
http://dxr.mozilla.org/mozilla-central/source/netwerk/protocol/rtsp/rtsp/APacketSource.cpp#73

This assertion checks the "config" parameter of MP4A payload format. If the number of digits of this parameters is not an even number, the assertion raises and causes crash.
Therefore, the root cause is a format error.

The format of MP4A is defined in RFC 6416.
http://tools.ietf.org/html/rfc6416
And, according to this RFC, this parameter is called AudioSpecificConfig, which is defined in:
[14496-3]  MPEG, "ISO/IEC International Standard 14496-3 - Coding of audio-visual objects, Part 3 Audio", 2009.
I tried to play the link specified in comment 0 by VLC player desktop, although VLC doesn't crash, it keeps trying and could not render anything. This also reveals there is format errors in the media source.
From the perspective of our RTSP client, we could turn this assertion into a normal error handle and report an error message such as "Format Error" to media decoder. Then the horrible crash could be avoided.

However, any similar format error checked by other assertions could still cause system crash.
And currently there are more than 300 such assertions in our RTSP implementation. It would be a big effort to convert all of them to error handles.
We could also turn off these assertions all at once to avoid crashes, but it will be much more difficult to debug format errors like this bug.

Steve and Vincent, do you have any idea/suggestion on this?
Flags: needinfo?(vchang)
Flags: needinfo?(sworkman)
I'm not sure if this helps, bug 1040984 has a similar crash.  Is that a dup?
(In reply to Ethan Tseng [:ethan] from comment #23)
If we can avoid crash assertions in release builds, that seems to be desirable. It looks like the assertion in question is a CHECK statement. I've seen that used throughout this code, so I don't think we can disable the underlying assertion. But maybe there's a way to call a FailGracefully function, a new function that would just shut the streaming down? Kind of like a graceful assertion? - This sounds like your idea to turn it into a normal error handler - what do you think?

Or change it to be something like NS_ASSERTION: it is more like a warning message in normal runtime, but can be setup to crash using the correct env var. That way you can still use it to debug format errors. This might be quicker than the other suggestion.

Long term, it would be good to consider fuzzing tests in the automated framework to find problems related to format errors. You could enable crashing for such tests - and in many cases, you would probably expect it, I think. But that's long term.
Flags: needinfo?(sworkman)
(In reply to Steve Workman [:sworkman] Offline until 7/30 from comment #25)
> (In reply to Ethan Tseng [:ethan] from comment #23)
> If we can avoid crash assertions in release builds, that seems to be
> desirable. It looks like the assertion in question is a CHECK statement.
> I've seen that used throughout this code, so I don't think we can disable
> the underlying assertion. But maybe there's a way to call a FailGracefully
> function, a new function that would just shut the streaming down? Kind of
> like a graceful assertion? - This sounds like your idea to turn it into a
> normal error handler - what do you think?
> 
> Or change it to be something like NS_ASSERTION: it is more like a warning
> message in normal runtime, but can be setup to crash using the correct env
> var. That way you can still use it to debug format errors. This might be
> quicker than the other suggestion.
> 
> Long term, it would be good to consider fuzzing tests in the automated
> framework to find problems related to format errors. You could enable
> crashing for such tests - and in many cases, you would probably expect it, I
> think. But that's long term.

Hi! Ethan,

Any action item? Thanks

--
Keven
Flags: needinfo?(ettseng)
(In reply to Keven Kuo [:kkuo] from comment #26)
> Hi! Ethan,
> Any action item? Thanks
> Keven

I will generate a patch to address this issue. Hopefully I can make it today.
Flags: needinfo?(ettseng)
Component: Video/Audio → RTSP
Product: Core → Firefox OS
Can we have a tool to generate RTSP and SDP attack packets? It may prevent most of the crashes if the coverage of the test cases is high.
Flags: needinfo?(vchang)
Attached patch bug-1038037-wip.patch (obsolete) — Splinter Review
A WIP patch that avoids the crash.
This is just a WIP patch that could avoid the crash in this specific scenario.
Instead of crash, an error message "Video playback aborted due to a network error" would be displayed.
Hi Steve,
Could you help to review this patch? It's short and simply replaces several assertions by if() checks.
Attachment #8461606 - Attachment is obsolete: true
Attachment #8462394 - Flags: review?(sworkman)
We found at least two format errors in the SDP packets from this server.

1. There is no newline in the last line of the SDP description.
   According to Section 5 in RFC 4566 (SDP: Session Description Protocol):
   "Text fields such as the session name and information are octet
   strings that may contain any octet with the exceptions of 0x00 (Nul),
   0x0a (ASCII newline), and 0x0d (ASCII carriage return).  The sequence
   CRLF (0x0d0a) is used to end a record, although parsers SHOULD be
   tolerant and also accept records terminated with a single newline
   character."
   
   The captured SDP packet reveals there is no newline character in the last line, which causes one of our assertions fail.

2. The format of the "config" parameter is incorrect. (also mentioned comment 21)
   Even if a newline character is added to the last line of SDP, the SDP would still fail in another assertion.
   RFC 6416 (RTP Payload Format for MPEG-4 Streams) defines this parameter in section 7.3:
   ""config": a hexadecimal representation of an octet string that
    expresses the audio payload configuration data "StreamMuxConfig",
    as defined in [14496-3]."

    I am not quite sure about the exact spec of StreamMuxConfig, but from our implementation we can tell the value of this parameter should contain even-number digits.
Whiteboard: [sprd333131][partner-blocker][b2g-crash] → [sprd333131][partner-blocker][b2g-crash] [p=2]
Target Milestone: --- → 2.1 S1 (1aug)
I encountered the same crash event today. FYI.
- https://crash-stats.mozilla.com/report/index/40b052b2-3ebd-4ed4-bb4f-69f632140725

Reproduced on flame (v2.1).
* Build information:
 - Gaia      c72257b2d27135bfcd68e89dd584182797784016
 - Gecko     https://hg.mozilla.org/mozilla-central/rev/616e6924cb0b
 - BuildID   20140724160202
 - Version   34.0a1
(In reply to William Hsu [:whsu] from comment #33)
> I encountered the same crash event today. FYI.
> -
> https://crash-stats.mozilla.com/report/index/40b052b2-3ebd-4ed4-bb4f-
> 69f632140725

William, what RTSP server did you use?
Did you use the links in comment0? Or you reproduce the crash on your own server?
> William, what RTSP server did you use?
> Did you use the links in comment0? Or you reproduce the crash on your own
> server?

I reproduced it via following RTSP test page. :D
- http://goo.gl/g0ORy6
(In reply to William Hsu [:whsu] from comment #35)
> I reproduced it via following RTSP test page. :D
> - http://goo.gl/g0ORy6

William,
I tried several links on this page but didn't encounter any crash.
Could you specify which link caused the crash?
And what about your reproduction rate?

Meanwhile, from the crash report you attached, I don't think the crash you encountered is the same as the one of this bug.
(In reply to Steve Workman [:sworkman] Offline until 7/30 from comment #25)
> If we can avoid crash assertions in release builds, that seems to be
> desirable. It looks like the assertion in question is a CHECK statement.
> I've seen that used throughout this code, so I don't think we can disable
> the underlying assertion. But maybe there's a way to call a FailGracefully
> function, a new function that would just shut the streaming down? Kind of
> like a graceful assertion? - This sounds like your idea to turn it into a
> normal error handler - what do you think?
> 
> Or change it to be something like NS_ASSERTION: it is more like a warning
> message in normal runtime, but can be setup to crash using the correct env
> var. That way you can still use it to debug format errors. This might be
> quicker than the other suggestion.

Hi Steve,
Thanks for your great advice!
Maybe we could apply different solutions depending to the conditions to be checked.
Anyway, I filed a new bug 1045062 to track this issue.

And for this bug, let's fix it by simply avoiding the crash.
How do you think?
(In reply to Ethan Tseng [:ethan] from comment #36)
> (In reply to William Hsu [:whsu] from comment #35)
> > I reproduced it via following RTSP test page. :D
> > - http://goo.gl/g0ORy6
> 
> William,
> I tried several links on this page but didn't encounter any crash.
> Could you specify which link caused the crash?
> And what about your reproduction rate?

OOPS...I forget the link. I encountered this crash event while I was verifying a streaming bug.
The reproduce rate of the crash event I mentioned is low (10%~20%), and it sometime happens on Browser app after you restart the device.

> 
> Meanwhile, from the crash report you attached, I don't think the crash you
> encountered is the same as the one of this bug.
Because I saw the same crash signature on the bug and it relates to streaming, so I posted the crash event here.
Do I need to submit the other bugs?

By the way, I saw a WIP patch on the Comment 31.
Maybe, I can merge it to my local branch to see if I still can reproduce it and also try the steps that Peipei posted.
What do you think? :D
(In reply to William Hsu [:whsu] from comment #38)
> Because I saw the same crash signature on the bug and it relates to
> streaming, so I posted the crash event here.
> Do I need to submit the other bugs?
> 
> By the way, I saw a WIP patch on the Comment 31.
> Maybe, I can merge it to my local branch to see if I still can reproduce it
> and also try the steps that Peipei posted.
> What do you think? :D

Sure, you can try my patch. However I don't think it would be helpful to your cases.

That patch avoids a crash caused by incorrect format of "MP4A-LATM" audio type.
And those media sources on your page don't have such issue.

Perhaps the crash you generated is another bug.
Summary: [dolphin][flame] b2g crash when open some streaming audio from browser → [dolphin][flame] B2G crash when opening malformed MP4A-LATM audio streaming over RTSP
(In reply to Ethan Tseng [:ethan] from comment #39)
> (In reply to William Hsu [:whsu] from comment #38)
> > Because I saw the same crash signature on the bug and it relates to
> > streaming, so I posted the crash event here.
> > Do I need to submit the other bugs?
> > 
> > By the way, I saw a WIP patch on the Comment 31.
> > Maybe, I can merge it to my local branch to see if I still can reproduce it
> > and also try the steps that Peipei posted.
> > What do you think? :D
> 
> Sure, you can try my patch. However I don't think it would be helpful to
> your cases.
> 
> That patch avoids a crash caused by incorrect format of "MP4A-LATM" audio
> type.
> And those media sources on your page don't have such issue.
> 
> Perhaps the crash you generated is another bug.

Thanks for the information and suggestion.
Okay. I skip my tests because the patch intent to fix "MP4A-LATM" audio type related issue.
Also, I will submit the other bug to trace the crash event I mentioned.
Thanks.
Comment on attachment 8462394 [details] [diff] [review]
Avoid crash from format error in MP4A-LATM

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

Some questions. Leaving review open.

::: netwerk/protocol/rtsp/rtsp/APacketSource.cpp
@@ +214,5 @@
>      return csd;
>  }
>  
>  sp<ABuffer> MakeAACCodecSpecificData(const char *params) {
> +    if (!strlen(params)) {

Let's just be extra defensive here and do:
if (!params || !strlen(params)) ...

@@ +225,3 @@
>  
>      sp<ABuffer> config = decodeHex(val);
> +    if (config == NULL) {

if (!config), no?

@@ +226,5 @@
>      sp<ABuffer> config = decodeHex(val);
> +    if (config == NULL) {
> +      return NULL;
> +    }
> +    if (config->size() < 4u) {

This is different. Previously, size must have been strictly 4u; now you're placing a minimum limit on it. Is this ok? Is there an upper bound you want to place on it?

@@ +484,5 @@
>          mFormat->setInt32(kKeyChannelCount, numChannels);
>  
>          sp<ABuffer> codecSpecificData =
>              MakeAACCodecSpecificData(params.c_str());
> +        if (codecSpecificData == NULL) {

This is new and doesn't replace an existing check. Please explain :)
Comment on attachment 8462394 [details] [diff] [review]
Avoid crash from format error in MP4A-LATM

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

::: netwerk/protocol/rtsp/rtsp/APacketSource.cpp
@@ +214,5 @@
>      return csd;
>  }
>  
>  sp<ABuffer> MakeAACCodecSpecificData(const char *params) {
> +    if (!strlen(params)) {

Sure!

@@ +225,3 @@
>  
>      sp<ABuffer> config = decodeHex(val);
> +    if (config == NULL) {

Android sp<T> cannot be converted to a boolean value.
But I can (and should) change this line to if (!config.get()) { }.
Thanks for reminder!

@@ +226,5 @@
>      sp<ABuffer> config = decodeHex(val);
> +    if (config == NULL) {
> +      return NULL;
> +    }
> +    if (config->size() < 4u) {

CHECK_GE() checks for "greater than", not "equal to".
#define CHECK_GE(x,y)   CHECK_OP(x,y,GE,>=)

@@ +484,5 @@
>          mFormat->setInt32(kKeyChannelCount, numChannels);
>  
>          sp<ABuffer> codecSpecificData =
>              MakeAACCodecSpecificData(params.c_str());
> +        if (codecSpecificData == NULL) {

Before my change, MakeAACCodecSpecificData() doesn't return error code. The 3 assertion checks could cause system crash if any of one fails.
For example, in the case of this bug, "CHECK(GetAttribute(params, "config", &val))" could fail because params is an empty string.
I just turns those assertions into if checks and return NULL as error code. So once 
MakeAACCodecSpecificData() returns NULL, it means the packet is malformed.

BTW, I will also change this line to "if (!codecSpecificData.get()) { }"
(In reply to Ethan Tseng [:ethan] from comment #42)
> Avoid crash from format error in MP4A-LATM
> Review of attachment 8462394 [details] [diff] [review]:
> -----------------------------------------------------------------

Oops! I should just reply your comments instead of commenting in patch review.
Hope you can get through it. Sorry for inconvenience.
Addressed issues in review comment.
Attachment #8462394 - Attachment is obsolete: true
Attachment #8462394 - Flags: review?(sworkman)
Attachment #8466011 - Flags: review?(sworkman)
Comment on attachment 8466011 [details] [diff] [review]
Avoid crash from format error in MP4A-LATM

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

Thanks Ethan!

re CHECK_GE, apologies; I hadn't read it correctly.

Looks good - r=me.
Attachment #8466011 - Flags: review?(sworkman) → review+
(In reply to Steve Workman [:sworkman] from comment #45)
> Thanks Ethan!
> re CHECK_GE, apologies; I hadn't read it correctly.
> Looks good - r=me.

Thanks Steve!
You are working at 2:30am? :p
Refresh comment "r=sworkman".
Attachment #8466011 - Attachment is obsolete: true
(In reply to Ethan Tseng [:ethan] from comment #46)
> (In reply to Steve Workman [:sworkman] from comment #45)
> > Thanks Ethan!
> > re CHECK_GE, apologies; I hadn't read it correctly.
> > Looks good - r=me.
> 
> Thanks Steve!
> You are working at 2:30am? :p

Haha! No! Working from Ireland yesterday and today ;) So 10:30am instead.
(In reply to Steve Workman [:sworkman] from comment #49)
> (In reply to Ethan Tseng [:ethan] from comment #46)
> > Thanks Steve!
> > You are working at 2:30am? :p
> Haha! No! Working from Ireland yesterday and today ;) So 10:30am instead.

Wow! Cool~
Enjoy your business trip. :)
Keywords: checkin-needed
https://hg.mozilla.org/integration/b2g-inbound/rev/1d99d18b86c5

Please be more conscientious about what platforms/testsuites you include in your Try pushes. "All" runs are almost always unnecessary and consume over 300hr of machine time, tying up slaves and contributing to backlog felt by all devs.
https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices
Keywords: checkin-needed
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #51)
> https://hg.mozilla.org/integration/b2g-inbound/rev/1d99d18b86c5
> Please be more conscientious about what platforms/testsuites you include in
> your Try pushes. "All" runs are almost always unnecessary and consume over
> 300hr of machine time, tying up slaves and contributing to backlog felt by
> all devs.
> https://wiki.mozilla.org/Sheriffing/How:To:Recommended_Try_Practices

Ryan, I apologize and thank for your reminder.
I will keep that in mind next time.
https://hg.mozilla.org/mozilla-central/rev/1d99d18b86c5
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/d5625dcbd7f1

Note that per the recent rule change announced in dev-gaia, 1.4 blockers don't have auto-approval to land on v2.0. Please nominate this patch for b2g32 uplift if you feel that it needs to land there as well. Sorry for the inconvenience :(
(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #54)
> https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/d5625dcbd7f1
> 
> Note that per the recent rule change announced in dev-gaia, 1.4 blockers
> don't have auto-approval to land on v2.0. Please nominate this patch for
> b2g32 uplift if you feel that it needs to land there as well. Sorry for the
> inconvenience :(

Actually this bug should not be a 1.4 blocker at all (because RTSP video is not enabled in v1.4).
It was marked as 1.4+ improperly.
But we need the fix in v2.0.
Flags: needinfo?(ettseng)
Comment on attachment 8466085 [details] [diff] [review]
Avoid crash from format error in MP4A-LATM

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: System crash while playing malformed MP4A audio over RTSP
Testing completed: On m-c
Risk to taking this patch (and alternatives if risky): No risk
String or UUID changes made by this patch: N/A
Attachment #8466085 - Flags: approval-mozilla-b2g32?
Attachment #8466085 - Flags: approval-mozilla-b2g32? → approval-mozilla-b2g32+
This issue has been verified successfully on Flame 2.0,2.1

See attachment: Verify_video.3gp
Reproducing rate: 0/5
flame2.1 build:
Gaia-Rev        5655269098c7e82254e56933f1af05b4abe2a2f3
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/86608c9389b5
Build-ID        20141204001201
Version         34.0
Flame 2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/ff1100ba2ab8
Build-ID        20141204000228
Version         32.0
Attached video Verify_video.3gp
You need to log in before you can comment on or make changes to this bug.