Closed Bug 726889 Opened 13 years ago Closed 13 years ago

rename nsMediaStream

Categories

(Core :: Audio/Video, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla13

People

(Reporter: roc, Assigned: roc)

References

Details

Attachments

(1 file, 1 obsolete file)

nsMediaStream conflicts with "MediaStream" DOM APIs. Let's rename it to mozilla::MediaResource/ChannelMediaResource/FileMediaResource.
Attached patch fix (obsolete) (deleted) — Splinter Review
Attachment #596904 - Flags: review?(cpearce)
Comment on attachment 596904 [details] [diff] [review] fix Review of attachment 596904 [details] [diff] [review]: ----------------------------------------------------------------- There's some variables called "stream" which should be "resource", e.g.: |MediaResource* stream = mDecoder->GetResource();|. May as well make them all "resource" for consistency. ::: content/media/nsMediaStream.cpp @@ +361,5 @@ > } > > struct CopySegmentClosure { > nsCOMPtr<nsIPrincipal> mPrincipal; > + ChannelMediaResource* mStream; Do you want to rename this to mResource for consistency? Maybe there's others too? @@ +537,4 @@ > { > NS_ASSERTION(NS_IsMainThread(), "Only call on main thread"); > > + ChannelMediaResource* stream = new ChannelMediaResource(aDecoder, nsnull, mURI); s/stream/resource/ ? ::: content/media/nsMediaStream.h @@ +409,5 @@ > public nsIInterfaceRequestor, > public nsIChannelEventSink > { > public: > + Listener(ChannelMediaResource* aStream) : mStream(aStream) {} s/mStream/mResource/ ? ::: content/media/nsBuiltinDecoderStateMachine.cpp @@ +1999,5 @@ > } > > // Check to see if we don't have enough data to play up to the next frame. > // If we don't, switch to buffering mode. > + MediaResource* stream = mDecoder->GetResource(); s/stream/resource/ ? A couple more of them below too. ::: content/media/nsMediaDecoder.h @@ +143,5 @@ > // point of the first frame of data. > // aStream is the media stream to use. Ownership of aStream passes to > // the decoder, even if Load returns an error. > // This is called at most once per decoder, after Init(). > + virtual nsresult Load(MediaResource* aStream, s/aStream/aResource/ ::: content/media/ogg/nsOggReader.cpp @@ +81,2 @@ > static PageSyncResult > +PageSync(MediaResource* aStream, aResource @@ +1074,2 @@ > static PageSyncResult > +PageSync(MediaResource* aStream, aResource @@ +1471,5 @@ > startTime = nsTheoraState::Time(&mTheoraInfo, granulepos); > NS_ASSERTION(startTime > 0, "Must have positive start time"); > } > else if (IsKnownStream(serial)) { > + // stream is not the theora or vorbis stream we're playing, s/stream/Stream/. This is not the stream you're looking for. ;) ::: content/media/raw/nsRawReader.cpp @@ +76,2 @@ > > + if (!ReadFromStream(resource, reinterpret_cast<PRUint8*>(&mMetadata), ReadFromResource @@ +202,5 @@ > !(header.packetID == 0xFF && header.codecID == RAW_ID /* "YUV" */)) { > return false; > } > > + if (!ReadFromStream(resource, buffer, length)) { ReadFromResource ::: content/media/webm/nsWebMReader.cpp @@ +450,5 @@ > } > if (tstamp_frames > decoded_frames) { > #ifdef DEBUG > PRInt64 usecs = 0; > + LOG(PR_LOG_DEBUG, ("WebMReader detected gap of %lld, %lld frames, in audio resource\n", s/resource/stream/. This is not the stream you're looking for.
Attachment #596904 - Flags: review?(cpearce) → review-
Attached patch fix v2 (deleted) — Splinter Review
All fixed.
Attachment #596904 - Attachment is obsolete: true
Attachment #596908 - Flags: review?(cpearce)
This might conflict with Blob support that Jonas was asking me about.
Comment on attachment 596908 [details] [diff] [review] fix v2 Review of attachment 596908 [details] [diff] [review]: ----------------------------------------------------------------- r+ with three nits picked. Looks good. ::: content/media/nsMediaCache.h @@ +191,5 @@ > * to try to avoid requesting seeks on such streams. > * > * nsMediaCache has a single internal monitor for all synchronization. > * This is treated as the lowest level monitor in the media code. So, > + * we must not acquire any nsMediaDecoder locks or MediaStream locks Did you mean "MediaResource locks"? @@ +359,5 @@ > // Returns true when all streams for this resource are suspended or their > // channel has ended. > // If aActiveStream is non-null, fills it with a pointer to a stream > // for this resource that is not suspended or ended. > + bool AreAllStreamsForResourceSuspended(MediaResource** aActiveStream); aActiveResource, update the comment too. ::: content/media/nsMediaDecoder.h @@ +143,5 @@ > // point of the first frame of data. > // aStream is the media stream to use. Ownership of aStream passes to > // the decoder, even if Load returns an error. > // This is called at most once per decoder, after Init(). > + virtual nsresult Load(MediaResource* aStream, aResource, update the comment.
Attachment #596908 - Flags: review?(cpearce) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: