818672 - Implement BufferMediaResource

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[(no longer active)]

This class implements a MediaResource which encapsulates an in-memory buffer.  I need this for my decodeAudioData implementation.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 688939 [details] [diff] [review]
Patch (v1)

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

Not sure what you should do in GetCurrentPrincipal(), Roc would know.

::: content/media/BufferMediaResource.h
@@ +23,5 @@
> +    mBuffer(aBuffer),
> +    mLength(aLength),
> +    mOffset(0)
> +  {
> +    MOZ_COUNT_CTOR(MediaResource);

Shouldn't this be MOZ_COUNT_CTOR(BufferMediaResource) ?

Comment 3

[(no longer active)]

(In reply to comment #2)
> ::: content/media/BufferMediaResource.h
> @@ +23,5 @@
> > +    mBuffer(aBuffer),
> > +    mLength(aLength),
> > +    mOffset(0)
> > +  {
> > +    MOZ_COUNT_CTOR(MediaResource);
> 
> Shouldn't this be MOZ_COUNT_CTOR(BufferMediaResource) ?

Of course.  I'll fix that when landing.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 688939 [details] [diff] [review]
Patch (v1)

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

::: content/media/BufferMediaResource.h
@@ +37,5 @@
> +  virtual void Resume() {}
> +  // Get the current principal for the channel
> +  virtual already_AddRefed<nsIPrincipal> GetCurrentPrincipal()
> +  {
> +    return nullptr;

I think you should pass in a principal to the constructor that gets used here.

@@ +91,5 @@
> +  virtual void Pin() {}
> +  virtual void Unpin() {}
> +  virtual double GetDownloadRate(bool* aIsReliable) { return 0.; }
> +  virtual int64_t GetLength() { return mLength; }
> +  virtual int64_t GetNextCachedData(int64_t aOffset) { return 0; }

We should treat everything as cached, so this should return aOffset.

@@ +92,5 @@
> +  virtual void Unpin() {}
> +  virtual double GetDownloadRate(bool* aIsReliable) { return 0.; }
> +  virtual int64_t GetLength() { return mLength; }
> +  virtual int64_t GetNextCachedData(int64_t aOffset) { return 0; }
> +  virtual int64_t GetCachedDataEnd(int64_t aOffset) { return 0; }

Should return mLength.

@@ +93,5 @@
> +  virtual double GetDownloadRate(bool* aIsReliable) { return 0.; }
> +  virtual int64_t GetLength() { return mLength; }
> +  virtual int64_t GetNextCachedData(int64_t aOffset) { return 0; }
> +  virtual int64_t GetCachedDataEnd(int64_t aOffset) { return 0; }
> +  virtual bool IsDataCachedToEndOfResource(int64_t aOffset) { return false; }

Should return true.

@@ +100,5 @@
> +  virtual nsresult ReadFromCache(char* aBuffer,
> +                                 int64_t aOffset,
> +                                 uint32_t aCount)
> +  {
> +    return NS_ERROR_FAILURE;

Should behave like Read.

@@ +116,5 @@
> +  }
> +
> +  virtual nsresult GetCachedRanges(nsTArray<MediaByteRange>& aRanges)
> +  {
> +    return NS_ERROR_FAILURE;

This should return a single range containing the entire resource.

Comment 5

[(no longer active)]

(In reply to comment #4)
> ::: content/media/BufferMediaResource.h
> @@ +37,5 @@
> > +  virtual void Resume() {}
> > +  // Get the current principal for the channel
> > +  virtual already_AddRefed<nsIPrincipal> GetCurrentPrincipal()
> > +  {
> > +    return nullptr;
> 
> I think you should pass in a principal to the constructor that gets used here.

What would that principal be?  I can accept that as an argument to the ctor, but it's still would be null.  Is that OK?

Comment 6

[(no longer active)]

Attachment: Patch (v2)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Ehsan Akhgari [:ehsan] from comment #5)
> What would that principal be?  I can accept that as an argument to the ctor,
> but it's still would be null.  Is that OK?

Anything accessing the contents of the resource must have a principal that subsumes the resource's principal. So use the principal of the window that called decode I guess.

Comment 8

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0acd2e86ec47

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0acd2e86ec47