Closed Bug 811381 Opened 7 years ago Closed 7 years ago

Remove ns prefix from media code

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla19

People

(Reporter: cpearce, Assigned: cpearce)

Details

Attachments

(5 files, 3 obsolete files)

We'd like to:
1. Flatten nsMediaDecoder and nsBuiltinDecoder into a single class.
2. Move media code into moilla namespace.
3. Remove the "ns" prefix from our class names.
Comment on attachment 681131 [details] [diff] [review]
Patch 1 v1: Flatten nsBultinDecoder and nsMediaDecoder into a single class.

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

::: content/media/nsBuiltinDecoder.cpp
@@ +1307,5 @@
>  ReentrantMonitor& nsBuiltinDecoder::GetReentrantMonitor() {
>    return mReentrantMonitor.GetReentrantMonitor();
>  }
>  
> +mozilla::layers::ImageContainer* nsBuiltinDecoder::GetImageContainer()

add "using namespace mozilla::layers;" to this file
Attachment #681131 - Flags: review?(roc) → review+
Comment on attachment 681134 [details] [diff] [review]
Patch 2 v1: Move content/media code into mozilla namespace

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

Can't we remove a bunch of the "typedef mozilla::T T;" typedefs in nsBuiltinDecoder (and maybe elsewhere)?
Yes, for class inside the mozilla namespace. I'll do that where possible.
Comment on attachment 681135 [details] [diff] [review]
Patch 3: Remove rs prefix, s/BuiltinDecoder/MediaDecoder/g

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

::: content/media/nsAudioStream.cpp
@@ +364,2 @@
>  #endif
> +  gAudioPrefsLock = new mozilla::Mutex("AudioStream::gAudioPrefsLock");

mozilla:: prefix should not be needed anymore
Attachment #681135 - Flags: review?(roc) → review+
With review comment addressed.
Attachment #681131 - Attachment is obsolete: true
Attachment #681161 - Flags: review+
Removed a bunch of mozilla::T T typedefs.
Attachment #681134 - Attachment is obsolete: true
Attachment #681134 - Flags: review?(roc)
Attachment #681164 - Flags: review?(roc)
Rebased, removed a bunch of other mozilla:: prefixes as well.
Attachment #681135 - Attachment is obsolete: true
Attachment #681165 - Flags: review+
I had a build failure in patch 1 on OSX [1]. Apparently clang can't handle including nsBuiltinDecoder.h in nsHTMLMediaElement.h, as clang gets confused between gfx::Point (which must be included by something that nsBuiltinDecoder.h includes) and a Point struct which is defined in MacTypes.h. So the solution is to not include nsBuiltinDecoder.h in nsHTMLMediaElement.

So do to that I need to extract the stuff needed by nsHTMLMediaElement that's in nsBuiltinDecoder.h (NextFrameStatus and MetadataTags) and move it into MediaDecoderOwner, which fits well, and is include that in nsHTMLMediaElement.h instead of nsBuiltinDecoder.h.

[1] https://tbpl.mozilla.org/php/getParsedLog.php?id=16999936&tree=Try#error1
Attachment #681373 - Flags: review?(roc)
Here's a version of the script I used modified so that people can run it over their patch queues to make rebasing easier.
Attachment #681600 - Attachment mime type: application/x-shellscript → text/plain
You need to log in before you can comment on or make changes to this bug.