Last Comment Bug 821396 - Fix build warnings in content/media/dash
: Fix build warnings in content/media/dash
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: unspecified
: All Linux
: -- normal (vote)
: mozilla20
Assigned To: Catalin Iordache
:
Mentors:
Depends on:
Blocks: buildwarning 819664
  Show dependency treegraph
 
Reported: 2012-12-13 10:50 PST by Catalin Iordache
Modified: 2012-12-23 13:04 PST (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
p2v1.patch (4.31 KB, patch)
2012-12-13 10:50 PST, Catalin Iordache
no flags Details | Diff | Splinter Review
whitespaces removed (4.17 KB, patch)
2012-12-15 06:02 PST, Catalin Iordache
cpearce: review+
Details | Diff | Splinter Review
added for checkin (4.31 KB, patch)
2012-12-17 08:09 PST, Catalin Iordache
no flags Details | Diff | Splinter Review

Description Catalin Iordache 2012-12-13 10:50:50 PST
Created attachment 691894 [details] [diff] [review]
p2v1.patch

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20121129151900

Steps to reproduce:

Fix warnings in content/media/dash/DASHDecoder.cpp and content/media/dash/DASHRepDecoder.cpp
Comment 1 Catalin Iordache 2012-12-13 10:56:11 PST
I'm a beginner developer. This is actually my second patch for Mozilla codebase, but I have to say that the first one wasn't relevant( bug 821269).

I hope this one is more relevant.
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2012-12-15 05:28:51 PST
Comment on attachment 691894 [details] [diff] [review]
p2v1.patch

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

::: content/media/dash/DASHRepDecoder.cpp
@@ +270,5 @@
>    // i.e. inactive rep decoders should only load metadata.
>    bool canDownloadData = mMainDecoder->IsDecoderAllowedToDownloadData(this);
>    if (canDownloadData) {
> +    for (uint32_t i = 0; i < mByteRanges.Length(); i++) {
> +      NS_ENSURE_FALSE(mByteRanges[i].IsNull(), NS_ERROR_NOT_INITIALIZED);    

You added some trailing whitespace here; please remove it.
Comment 3 Catalin Iordache 2012-12-15 06:02:36 PST
Created attachment 692590 [details] [diff] [review]
whitespaces removed
Comment 4 Chris Pearce (:cpearce) 2012-12-16 14:09:46 PST
Comment on attachment 692590 [details] [diff] [review]
whitespaces removed

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

::: content/media/dash/DASHDecoder.cpp
@@ +697,5 @@
>  
>        // Do Stream Switching here before loading next bytes.
>        // Audio stream switching not supported.
>        if (aRepDecoder == VideoRepDecoder() &&
> +          (uint32_t)mVideoSubsegmentIdx < VideoRepDecoder()->GetNumDataByteRanges()) {

Make this:
uint32_t(mVideoSubsegmentIdx)

@@ +938,5 @@
>    NS_ENSURE_TRUE(toDecoderIdx < mVideoRepDecoders.Length(),
>                   NS_ERROR_ILLEGAL_VALUE);
>  
>    // Notify reader and sub decoders and do the switch.
> +  if (toDecoderIdx != (uint32_t)mVideoRepDecoderIdx) {

Make this:
uint32_t(mVideoRepDecoderIdx)
Comment 5 Catalin Iordache 2012-12-17 08:09:55 PST
Created attachment 692958 [details] [diff] [review]
added for checkin
Comment 6 Ryan VanderMeulen [:RyanVM] 2012-12-23 08:24:15 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba26ddd4739b
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-12-23 13:04:56 PST
https://hg.mozilla.org/mozilla-central/rev/ba26ddd4739b

Note You need to log in before you can comment on or make changes to this bug.