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

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RTSP, defect)

Description

[Peipei Cheng (needinfo if you need my action)]

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

Comment 1

[Peipei Cheng (needinfo if you need my action)]

The crash report looks like Bug 1035755 - [V1.4][Camera][Dolphin] Repeatedly tap video record button causes app crash

Comment 2

[Peipei Cheng (needinfo if you need my action)]

(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

Comment 3

[Peipei Cheng (needinfo if you need my action)]

Flame v1.3 base image also has this problem.

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

We need a crash report with a build that's associated with symbols.  Please try on flame 1.4 build.

Comment 5

[Peipei Cheng (needinfo if you need my action)]

I got a new crash report on flame debug version: https://crash-stats.mozilla.com/report/index/f38a6161-507d-4bce-808d-763192140715

Comment 6

[Peipei Cheng (needinfo if you need my action)]

seems the same issue with bug: Bug 1035755 - [V1.4][Camera][Dolphin] Repeatedly tap video record button causes app crash

Comment 8

[Peipei Cheng (needinfo if you need my action)]

(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.

Comment 9

[Peipei Cheng (needinfo if you need my action)]

I will double check after bug 1035755 is fixed.

Comment 10

[Pi Wei Cheng [:piwei] (inactive)]

qawanted request has been fulfilled on comment 5.

Comment 11

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Bug 1035755 landed yesterday.  This should be in today's build to test for flame.

Comment 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

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.

Comment 13

[Peipei Cheng (needinfo if you need my action)]

I also tested on flame and dolphin today. This crash still happens.

Comment 14

[Wayne Chang [:wchang]]

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.

Comment 15

[Blake Wu [:bwu][:blakewu]]

I am going to investigate this bug. This can be reproed on my Nexus 5 as well.

Comment 16

[Blake Wu [:bwu][:blakewu]]

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 ?? ()

Comment 17

[Benjamin Chen [:bechen]]

Hi Ethan:
As discussed, please check the sdp packet first and the solution might be similar to Bug 1009497.

Comment 18

[Blake Wu [:bwu][:blakewu]]

Thanks! Benjamin and Ethan.

Comment 19

[Ethan Tseng [: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.

Comment 20

[Ethan Tseng [:ethan]]

Attachment: bug-1038037-rtsp-sdp.pcap

Captured RTSP packets.

Comment 21

[Ethan Tseng [:ethan]]

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.

Comment 22

[Ethan Tseng [:ethan]]

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.

Comment 23

[Ethan Tseng [:ethan]]

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?

Comment 24

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I'm not sure if this helps, bug 1040984 has a similar crash.  Is that a dup?

Comment 25

[Steve Workman [:sworkman] (INACTIVE)]

(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.

Comment 26

[Keven Kuo [:kkuo]]

(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

Comment 27

[Ethan Tseng [:ethan]]

(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.

Comment 28

[Vincent Chang[:vchang][changyihsin]]

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.

Comment 29

[Ethan Tseng [:ethan]]

Attachment: bug-1038037-wip.patch

A WIP patch that avoids the crash.

Comment 30

[Ethan Tseng [:ethan]]

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.

Comment 31

[Ethan Tseng [:ethan]]

Attachment: Avoid crash from format error in MP4A-LATM

Hi Steve,
Could you help to review this patch? It's short and simply replaces several assertions by if() checks.

Comment 32

[Ethan Tseng [:ethan]]

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.

Comment 33

[William Hsu [:whsu]]

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

Comment 34

[Ethan Tseng [:ethan]]

(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?

Comment 35

[William Hsu [:whsu]]

> 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

Comment 36

[Ethan Tseng [:ethan]]

(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.

Comment 37

[Ethan Tseng [:ethan]]

(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?

Comment 38

[William Hsu [:whsu]]

(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

Comment 39

[Ethan Tseng [:ethan]]

(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.

Comment 40

[William Hsu [:whsu]]

(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 41

[Steve Workman [:sworkman] (INACTIVE)]

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 42

[Ethan Tseng [:ethan]]

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()) { }"

Comment 43

[Ethan Tseng [:ethan]]

(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.

Comment 44

[Ethan Tseng [:ethan]]

Attachment: Avoid crash from format error in MP4A-LATM

Addressed issues in review comment.

Comment 45

[Steve Workman [:sworkman] (INACTIVE)]

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.

Comment 46

[Ethan Tseng [:ethan]]

(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

Comment 47

[Ethan Tseng [:ethan]]

Attachment: Avoid crash from format error in MP4A-LATM

Refresh comment "r=sworkman".

Comment 48

[Ethan Tseng [:ethan]]

The result of Try server:
https://tbpl.mozilla.org/?tree=Try&rev=2cf3125a0238

Comment 49

[Steve Workman [:sworkman] (INACTIVE)]

(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.

Comment 50

[Ethan Tseng [:ethan]]

(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. :)

Comment 51

[Ryan VanderMeulen [:RyanVM]]

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

Comment 52

[Ethan Tseng [:ethan]]

(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.

Comment 53

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1d99d18b86c5

Comment 54

[Ryan VanderMeulen [:RyanVM]]

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 :(

Comment 55

[Ethan Tseng [:ethan]]

(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.

Comment 56

[Ethan Tseng [:ethan]]

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

Comment 57

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c393de5ce227

Comment 58

[Paladin]

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

Comment 59

[Paladin]

Attachment: Verify_video.3gp