Peipei Cheng (needinfo if you need my action) Reporter
	Description • 10 years ago

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

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 1 • 10 years ago

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

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 2 • 10 years ago

(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

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 3 • 10 years ago

Flame v1.3 base image also has this problem.

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 4 • 10 years ago

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

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 5 • 10 years ago

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

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 6 • 10 years ago

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

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 8 • 10 years ago

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

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 9 • 10 years ago

I will double check after bug 1035755 is fixed.

	Pi Wei Cheng [:piwei] (inactive)
	Comment 10 • 10 years ago

qawanted request has been fulfilled on comment 5.

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 11 • 10 years ago

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

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 12 • 10 years ago

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.

	Peipei Cheng (needinfo if you need my action) Reporter
	Comment 13 • 10 years ago

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

	Wayne Chang [:wchang]
	Comment 14 • 10 years ago

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.

	Blake Wu [:bwu][:blakewu]
	Comment 15 • 10 years ago

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

	Blake Wu [:bwu][:blakewu]
	Comment 16 • 10 years ago

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

	Benjamin Chen [:bechen]
	Comment 17 • 10 years ago

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

	Blake Wu [:bwu][:blakewu]
	Comment 18 • 10 years ago

Thanks! Benjamin and Ethan.

	Ethan Tseng [:ethan] Assignee
	Comment 19 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 20 • 10 years ago

Captured RTSP packets.

	Ethan Tseng [:ethan] Assignee
	Comment 21 • 10 years ago

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.

	Ethan Tseng [:ethan] Assignee
	Comment 22 • 10 years ago

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.

	Ethan Tseng [:ethan] Assignee
	Comment 23 • 10 years ago

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?

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 24 • 10 years ago

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

	Steve Workman [:sworkman] (INACTIVE)
	Comment 25 • 10 years ago

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

	Keven Kuo [:kkuo]
	Comment 26 • 10 years ago

(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

	Ethan Tseng [:ethan] Assignee
	Comment 27 • 10 years ago

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

	Vincent Chang[:vchang][changyihsin]
	Comment 28 • 10 years ago

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.

	Ethan Tseng [:ethan] Assignee
	Comment 29 • 10 years ago

A WIP patch that avoids the crash.

	Ethan Tseng [:ethan] Assignee
	Comment 30 • 10 years ago

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.

	Ethan Tseng [:ethan] Assignee
	Comment 31 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 32 • 10 years ago

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.

	William Hsu [:whsu]
	Comment 33 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 34 • 10 years ago

(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 Hsu [:whsu]
	Comment 35 • 10 years ago

> 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

	Ethan Tseng [:ethan] Assignee
	Comment 36 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 37 • 10 years ago

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

	William Hsu [:whsu]
	Comment 38 • 10 years ago

(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

	Ethan Tseng [:ethan] Assignee
	Comment 39 • 10 years ago

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

	William Hsu [:whsu]
	Comment 40 • 10 years ago

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

	Steve Workman [:sworkman] (INACTIVE)
	Comment 41 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 42 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 43 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 44 • 10 years ago

Addressed issues in review comment.

	Steve Workman [:sworkman] (INACTIVE)
	Comment 45 • 10 years ago

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.

	Ethan Tseng [:ethan] Assignee
	Comment 46 • 10 years ago

(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

	Ethan Tseng [:ethan] Assignee
	Comment 47 • 10 years ago

Refresh comment "r=sworkman".

	Ethan Tseng [:ethan] Assignee
	Comment 48 • 10 years ago

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

	Steve Workman [:sworkman] (INACTIVE)
	Comment 49 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 50 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 51 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 52 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 53 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 54 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 55 • 10 years ago

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

	Ethan Tseng [:ethan] Assignee
	Comment 56 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 57 • 10 years ago

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

	Paladin
	Comment 58 • 10 years ago

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