Add MediaSourceDemuxer, a MediaDataDemuxer.

RESOLVED FIXED in Firefox 41

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(11 attachments, 12 obsolete attachments)

2.60 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
1.34 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
1.16 KB, patch
cajbir
: review+
Details | Diff | Splinter Review
1.54 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
7.76 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
37.51 KB, patch
Details | Diff | Splinter Review
15.64 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
3.99 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
1.42 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
6.16 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
2.88 KB, patch
mattwoodrow
: review+
Details | Diff | Splinter Review
(Assignee)

Description

4 years ago
This is required to use the new TrackBuffersManager introduced in bug 1119208
(Assignee)

Updated

4 years ago
Blocks: 1171314
(Assignee)

Updated

4 years ago
Summary: Add MediaSourceDemuxer a MediaDataDemuxer. → Add MediaSourceDemuxer, a MediaDataDemuxer.
(Assignee)

Updated

4 years ago
Blocks: 1119208
No longer depends on: 1119208
(Assignee)

Updated

4 years ago
Blocks: 1171379
(Assignee)

Comment 1

4 years ago
Created attachment 8615676 [details] [diff] [review]
P1. Add MediaDataDemuxer::IsThreadSafe() method

Add MediaDataDemuxer::IsThreadSafe() method. The new media source demuxer can't support cloning. Cloning() works if you can share the MediaResource object, which in the case of media source, we don't have
Attachment #8615676 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
(Assignee)

Comment 2

4 years ago
Created attachment 8615677 [details] [diff] [review]
P2. Handles errors when skipping to next keyframe. c=cpearce

Handle EOS and WAITING_FOR_DATA during skip to next key frame. This is what caused occasional stalls in the facebook MSE video, or the video not playing to the end.
Attachment #8615677 - Flags: review?(cpearce)
(Assignee)

Comment 3

4 years ago
Created attachment 8615687 [details] [diff] [review]
Part3. Integrates new MSE

The MediaSourceDemuxer object.
Attachment #8615687 - Flags: review?(cpearce)
(Assignee)

Comment 4

4 years ago
Created attachment 8615697 [details] [diff] [review]
P3. Add MediaSourceDemuxer object

Rebase. Change commit comment
Attachment #8615697 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8615687 - Attachment is obsolete: true
Attachment #8615687 - Flags: review?(cpearce)
(Assignee)

Comment 5

4 years ago
Created attachment 8615846 [details] [diff] [review]
P4. Don't compute start time for MSE

Make media source reference time be 0 as per spec. Remove redundant virtual keyword when override is specified.
Attachment #8615846 - Flags: review?(cpearce)
(Assignee)

Comment 6

4 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #1)
> Created attachment 8615676 [details] [diff] [review]
> P1. Add MediaDataDemuxer::IsThreadSafe() method
> 
> Add MediaDataDemuxer::IsThreadSafe() method. The new media source demuxer
> can't support cloning. Cloning() works if you can share the MediaResource
> object, which in the case of media source, we don't have

This is only required due to how GetBuffered() is working. Bobby is working on removing the multi-thread access and using the state mirroring mechanism. Once this is done, we will be able to remove completely the demuxer cloning (there can be only one!)
(Assignee)

Comment 7

4 years ago
Created attachment 8617143 [details] [diff] [review]
P1. Add MediaDataDemuxer::IsThreadSafe() method

Rebase
Attachment #8617143 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8615676 - Attachment is obsolete: true
Attachment #8615676 - Flags: review?(cpearce)
(Assignee)

Comment 8

4 years ago
Created attachment 8617144 [details] [diff] [review]
P2. Handles errors when skipping to next keyframe. c=cpearce

Rebase
Attachment #8617144 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8615677 - Attachment is obsolete: true
Attachment #8615677 - Flags: review?(cpearce)
(Assignee)

Comment 9

4 years ago
Created attachment 8617148 [details] [diff] [review]
P3. Add MediaSourceDemuxer object

rebase
Attachment #8617148 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8615697 - Attachment is obsolete: true
Attachment #8615697 - Flags: review?(cpearce)
(Assignee)

Comment 10

4 years ago
Created attachment 8617149 [details] [diff] [review]
P4. Don't compute start time for MSE

rebase
Attachment #8617149 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8615846 - Attachment is obsolete: true
Attachment #8615846 - Flags: review?(cpearce)
(Assignee)

Comment 11

4 years ago
Created attachment 8617151 [details] [diff] [review]
P5. Force update of buffered range after endOfStream

Force update of buffered range cache when calling endofstream
Attachment #8617151 - Flags: review?(cajbir.bugzilla)
(Assignee)

Comment 12

4 years ago
Created attachment 8617155 [details] [diff] [review]
P6. Use ProxyMediaCall and remove use of monitor

Use ProxyMediaCall and remove need for monitor.
Attachment #8617155 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Depends on: 1171330

Updated

4 years ago
Attachment #8617151 - Flags: review?(cajbir.bugzilla) → review+
(Assignee)

Comment 13

4 years ago
Created attachment 8617182 [details] [diff] [review]
P7. Ensure we have all sourcebuffer ready

Ensure all source buffers are ready before resolving ReadMetadata(). Only recalculate the next keyframe time when needed
Attachment #8617182 - Flags: review?(cpearce)
(Assignee)

Comment 14

4 years ago
Created attachment 8617193 [details] [diff] [review]
P7. Ensure we have all sourcebuffer ready

Ensure all source buffers are ready before resolving ReadMetadata(). Only recalculate the next keyframe time when needed
Attachment #8617193 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8617182 - Attachment is obsolete: true
Attachment #8617182 - Flags: review?(cpearce)
(Assignee)

Comment 15

4 years ago
Created attachment 8617194 [details] [diff] [review]
P8. Ensure we will always decode available data

Only clear decoder.mInputExhausted once we feed data to the decoder. This ensure that we never stop attempting to decode while the decoder is awaiting for content. The flag was cleared as we were attempting to demux, demux which could have failed.
Attachment #8617194 - Flags: review?(cpearce)
(Assignee)

Comment 16

4 years ago
Created attachment 8617728 [details] [diff] [review]
P9. Only allow seeking if we have target time

Only allow seeking if we have the target data. We make an exception when seeking to 0, as it really means 'to the first frame' and a video may not exactly starts at 0.
Attachment #8617728 - Flags: review?(cpearce)
Attachment #8617143 - Flags: review?(cpearce) → review+
Attachment #8617144 - Flags: review?(cpearce) → review+
Comment on attachment 8617148 [details] [diff] [review]
P3. Add MediaSourceDemuxer object

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

Please replace your use of "auto" with the actual types, so that the code is easier to read and understand. Code is read more than its written, so optimize for readability. "auto" is ok when it's not important what the type is, like for iterators, or it's obvious.

::: dom/media/mediasource/MediaSourceDecoder.h
@@ +111,5 @@
>    // calls Attach/DetachMediaSource on this decoder to set and clear
>    // mMediaSource.
>    dom::MediaSource* mMediaSource;
> +  nsRefPtr<MediaDecoderReader> mReader;
> +  bool mFormatReader;

Please call this mIsUsingFormatReader, or mHasFormatReader, or some other "mIs..." or "mHas..." pattern, so it's obvious from the variable name that this is a boolean.

::: dom/media/mediasource/MediaSourceDemuxer.cpp
@@ +55,5 @@
> +{
> +  mMonitor.AssertCurrentThreadOwns();
> +
> +  // Reset our current content.
> +//  mInfo = MediaInfo();

You shouldn't be checking in commented out code. And if you were, the indent is wrong.

@@ +297,5 @@
> +  }
> +  const auto& track = mManager->GetTrackBuffer(mType);
> +  TimeUnit lastKeyFrameTime;
> +  uint32_t lastKeyFrameIndex = 0;
> +  for (uint32_t i = 0; i < track.Length(); i++) {

Is it worth using (or at least considering using) a bisection search here instead? nsTArray has bisection search in-built.

@@ +352,5 @@
> +  int32_t parsed = 0;
> +  const auto& track = mManager->GetTrackBuffer(mType);
> +  for (uint32_t i = mNextSampleIndex; i < track.Length(); i++) {
> +    const auto& sample = track[i];
> +    if (sample->mKeyframe && sample->mTime >= aTimeThreadshold.ToMicroseconds()) {

It's a shame that sample->mTime here is in microseconds rather than as a TimeUnit so you still need to explicitly convert the time units.

But I can't actually tell what type |sample| is, and therefore what the type of mTime is, because you're using auto here.

@@ +394,5 @@
> +    return nullptr;
> +  }
> +
> +  if (mNextSampleIndex) {
> +    auto& sample = track[mNextSampleIndex];

Because you're using auto here it's impossible to tell whether |sample| is a refcounted type.

In general, you should only use auto where it's obvious what the type is, or the type is template-hell, like a std::iterator.

::: dom/media/mediasource/MediaSourceDemuxer.h
@@ +25,5 @@
> +{
> +public:
> +  explicit MediaSourceDemuxer();
> +
> +  virtual nsRefPtr<InitPromise> Init() override;

"Method declarations must use at most one of the following keywords: "virtual", "override", or "final"."
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style

@@ +122,5 @@
> +  media::TimeUnit mNextRandomAccessPoint;
> +  MediaPromiseHolder<SeekPromise> mSeekPromise;
> +  MediaPromiseHolder<SamplesPromise> mSamplePromise;
> +  MediaPromiseHolder<SkipAccessPointPromise> mSkipPromise;
> +  Atomic<bool> mDetached;

Does this really need to be atomic?
Attachment #8617148 - Flags: review?(cpearce) → review+
(Assignee)

Comment 18

4 years ago
(In reply to Chris Pearce (:cpearce) from comment #17)
> Comment on attachment 8617148 [details] [diff] [review]
> P3. Add MediaSourceDemuxer object
> 

> 
> ::: dom/media/mediasource/MediaSourceDemuxer.cpp
> @@ +55,5 @@
> > +{
> > +  mMonitor.AssertCurrentThreadOwns();
> > +
> > +  // Reset our current content.
> > +//  mInfo = MediaInfo();
> 
> You shouldn't be checking in commented out code. And if you were, the indent
> is wrong.

this is fixed in patch 8.

> 
> @@ +297,5 @@
> > +  }
> > +  const auto& track = mManager->GetTrackBuffer(mType);
> > +  TimeUnit lastKeyFrameTime;
> > +  uint32_t lastKeyFrameIndex = 0;
> > +  for (uint32_t i = 0; i < track.Length(); i++) {
> 
> Is it worth using (or at least considering using) a bisection search here
> instead? nsTArray has bisection search in-built.
> 

it could be used yes... by using a reference though, we avoid creating unnecessary copy or new refptr.

> It's a shame that sample->mTime here is in microseconds rather than as a
> TimeUnit so you still need to explicitly convert the time units.

need to change the MediaData object, it's on my TODO list.

> 
> But I can't actually tell what type |sample| is, and therefore what the type
> of mTime is, because you're using auto here.
> 
> @@ +394,5 @@
> > +    return nullptr;
> > +  }
> > +
> > +  if (mNextSampleIndex) {
> > +    auto& sample = track[mNextSampleIndex];
> 
> Because you're using auto here it's impossible to tell whether |sample| is a
> refcounted type.

it's a reference, so it really doesn't matter if it's refcounted or not...
I guess we can't tell it's a pointer though.

will change.

> 
> In general, you should only use auto where it's obvious what the type is, or
> the type is template-hell, like a std::iterator.
> 
> ::: dom/media/mediasource/MediaSourceDemuxer.h
> @@ +25,5 @@
> > +{
> > +public:
> > +  explicit MediaSourceDemuxer();
> > +
> > +  virtual nsRefPtr<InitPromise> Init() override;
> 
> "Method declarations must use at most one of the following keywords:
> "virtual", "override", or "final"."
> https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style
> 

yeah, that is fixed in a later patch... will shift the changes and rebase.

> @@ +122,5 @@
> > +  media::TimeUnit mNextRandomAccessPoint;
> > +  MediaPromiseHolder<SeekPromise> mSeekPromise;
> > +  MediaPromiseHolder<SamplesPromise> mSamplePromise;
> > +  MediaPromiseHolder<SkipAccessPointPromise> mSkipPromise;
> > +  Atomic<bool> mDetached;
> 
> Does this really need to be atomic?

well, it was accessed from two different threads, and it's easier to deal with atomics than monitors.
having said that, all of this part is gone in a later patch.
(Assignee)

Comment 19

4 years ago
(In reply to Chris Pearce (:cpearce) from comment #17)
> Comment on attachment 8617148 [details] [diff] [review]
> P3. Add MediaSourceDemuxer object
> 
> Review of attachment 8617148 [details] [diff] [review]:

> Is it worth using (or at least considering using) a bisection search here
> instead? nsTArray has bisection search in-built.

I'll take this in bug 1171760.
(Assignee)

Comment 20

4 years ago
Created attachment 8617791 [details] [diff] [review]
P3. Add MediaSourceDemuxer object

Rebasing. Addressed comments. Carrying r+
(Assignee)

Updated

4 years ago
Attachment #8617148 - Attachment is obsolete: true
(Assignee)

Comment 21

4 years ago
Created attachment 8617792 [details] [diff] [review]
P4. Don't compute start time for MSE

Rebasing due to changes in P3
Attachment #8617792 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8617149 - Attachment is obsolete: true
Attachment #8617149 - Flags: review?(cpearce)
(Assignee)

Comment 22

4 years ago
Created attachment 8617798 [details] [diff] [review]
P3. Add MediaSourceDemuxer object

remove now useless variables.
(Assignee)

Updated

4 years ago
Attachment #8617791 - Attachment is obsolete: true
(Assignee)

Comment 23

4 years ago
Created attachment 8617802 [details] [diff] [review]
P6. Use ProxyMediaCall and remove use of monitor

Rebase.
Attachment #8617802 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8617155 - Attachment is obsolete: true
Attachment #8617155 - Flags: review?(cpearce)
(Assignee)

Comment 24

4 years ago
Created attachment 8617808 [details] [diff] [review]
P7. Ensure we have all sourcebuffer ready

Rebase.
Attachment #8617808 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8617193 - Attachment is obsolete: true
Attachment #8617193 - Flags: review?(cpearce)
(Assignee)

Comment 25

4 years ago
Created attachment 8617811 [details] [diff] [review]
P9. Only allow seeking if we have target time

Rebase
Attachment #8617811 - Flags: review?(cpearce)
(Assignee)

Updated

4 years ago
Attachment #8617728 - Attachment is obsolete: true
Attachment #8617728 - Flags: review?(cpearce)
Comment on attachment 8617194 [details] [diff] [review]
P8. Ensure we will always decode available data

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

::: dom/media/MediaFormatReader.cpp
@@ +875,5 @@
>    }
>    decoder.mQueuedSamples.Clear();
> +
> +  // We have serviced the decoder's request for more data.
> +  decoder.mInputExhausted = false;

Does this fix a bug inherited from MP4Reader? I'm wondering if there's a bug there on Aurora/Beta that we could fix.
Attachment #8617194 - Flags: review?(cpearce) → review+
Comment on attachment 8617792 [details] [diff] [review]
P4. Don't compute start time for MSE

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

::: dom/media/MediaDataDemuxer.h
@@ +101,5 @@
>  
> +  // Indicate to MediaFormatReader if it should compute the start time
> +  // of the demuxed data. If true (default) the first sample returned will be
> +  // used as reference time base.
> +  virtual bool ComputeStartTime() const { return true; }

You have two functions, MediaDataDemuxer::ComputeStartTime() and MediaFormatReader::ComputeStartTime() and they do different things but have the same name.

Please call this "MustComputeStartTime()" or something like that which differentiates that this function does not compute the start time, but in fact tells you that you *need* to compute the start time.
Attachment #8617792 - Flags: review?(cpearce) → review+
(In reply to Chris Pearce (:cpearce) from comment #27)
> Comment on attachment 8617792 [details] [diff] [review]
> P4. Don't compute start time for MSE

And it would have been easier to review if you did the virtual/override changes in a separate patch.
(Assignee)

Comment 29

4 years ago
(In reply to Chris Pearce (:cpearce) from comment #28)

(In reply to Chris Pearce (:cpearce) from comment #26)
> Comment on attachment 8617194 [details] [diff] [review]
> P8. Ensure we will always decode available data
> 
> Review of attachment 8617194 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/MediaFormatReader.cpp
> @@ +875,5 @@
> >    }
> >    decoder.mQueuedSamples.Clear();
> > +
> > +  // We have serviced the decoder's request for more data.
> > +  decoder.mInputExhausted = false;
> 
> Does this fix a bug inherited from MP4Reader? I'm wondering if there's a bug
> there on Aurora/Beta that we could fix.

Hmmm... indeed, MP4Reader would have the same issue.
Though the call to GetSample() is blocking and less likely to fail (as MediaSourceReader only ask for data if it *knows* the reader has the data.
Attachment #8617802 - Flags: review?(cpearce) → review+
Comment on attachment 8617808 [details] [diff] [review]
P7. Ensure we have all sourcebuffer ready

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

r+, but I'd like an answer on my question/comment regarding crypto being overwritten.

::: dom/media/mediasource/MediaSourceDemuxer.cpp
@@ +56,3 @@
>    MonitorAutoLock mon(mMonitor);
>  
> +  bool haveSourceBufferNotReady = false;

Maybe call that haveEmptySourceBuffer, so that you don't have to return a double negative below to improve understandability.

@@ +69,5 @@
>        mInfo.mVideo = info.mVideo;
>        mVideoTrack = sourceBuffer;
>      }
>      if (info.IsEncrypted() && !mInfo.IsEncrypted()) {
>        mInfo.mCrypto = info.mCrypto;

Does this line "mInfo.mCrypto = info.mCrypto" mean if we have two source buffers with different crypto info, we'll overwrite the crypto info from the first one we encounter with the latter? Isn't that wrong?
Attachment #8617808 - Flags: review?(cpearce) → review+
Attachment #8617811 - Flags: review?(cpearce) → review+
(Assignee)

Comment 31

4 years ago
(In reply to Chris Pearce (:cpearce) from comment #30)

> Does this line "mInfo.mCrypto = info.mCrypto" mean if we have two source
> buffers with different crypto info, we'll overwrite the crypto info from the
> first one we encounter with the latter? Isn't that wrong?

Actually, we have the problem that this value is only read during MediaDecoderReader::ReadMetadata(). With the existing MSE code, we have multiple sub-readers. One per init segment. So we always get one call to ReadMetadata() per crypto entry.

But with the new MSE and its single vector of samples, we have no way to pass the updated crypto back to the MediaFormatReader() each time we have an appendBuffer with new encrypted data.
So all we want to do here, is tell the MediaFormatReader() that there's encrypted data to worry about, regardless of which source buffer.

The handling of encrypted/key event would instead be done at the TrackBuffersManager level which will directly call MediaSourceDecoder and let it fire the encrypted event each time a new init segment is read.

When a new init segment is appended to the source buffer. If the future data is encrypted, then DispatchEncrypted() will be called on the MediaSourceDecoder's owner and MediaFormatReader doesn't have to worry about it any longer (but we do want it to create a CDM Wrapper, hence why we tell it mInfo is encrypted)

I'll have that in another bug.
(Assignee)

Comment 32

4 years ago
Created attachment 8620755 [details] [diff] [review]
P10. Add MediaDataDemuxer::GetSamplesMayBlock() method

Add GetSamplesMayBlock() and implement MediaFormatReader::UseBufferingHeuristics()
Attachment #8620755 - Flags: review?(cpearce)
Attachment #8620755 - Flags: review?(cpearce) → review+
(Assignee)

Comment 33

4 years ago
Created attachment 8620810 [details] [diff] [review]
P11. Only perform fuzz search on the interval's start

We need to perform our fuzzy search (the timeintervals has a fuzz that is half the frame duration) on the start of the interval only and always exclude the end
Attachment #8620810 - Flags: review?(matt.woodrow)
(Assignee)

Updated

4 years ago
Blocks: 1173657
Attachment #8620810 - Flags: review?(matt.woodrow) → review+
(Assignee)

Comment 36

4 years ago
Missed one:
(lldb) bt
* thread #1: tid = 0x13e697, 0x000000010675843c XUL`mozilla::MediaSourceReader::GetBuffered(this=0x0000000130311800) + 140 at MediaSourceReader.cpp:1013, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000010675843c XUL`mozilla::MediaSourceReader::GetBuffered(this=0x0000000130311800) + 140 at MediaSourceReader.cpp:1013
    frame #1: 0x000000010674ce6f XUL`mozilla::MediaSourceDecoder::GetSeekable(this=0x0000000125382640) + 367 at MediaSourceDecoder.cpp:90
  * frame #2: 0x0000000106448a5e XUL`mozilla::dom::HTMLMediaElement::Seek(this=0x0000000135e43800, aTime=4884.0720000000001, aSeekType=Accurate, aRv=0x00007fff5fbf49e8) + 942 at HTMLMediaElement.cpp:1491
    frame #3: 0x0000000106448fbe XUL`mozilla::dom::HTMLMediaElement::SetCurrentTime(this=0x0000000135e43800, aCurrentTime=4884.0720000000001, aRv=0x00007fff5fbf49e8) + 46 at HTMLMediaElement.cpp:1401
    frame #4: 0x0000000105f9259a XUL`mozilla::dom::HTMLMediaElementBinding::set_currentTime(cx=0x00000001303de850, obj=Handle<JSObject *> at 0x00007fff5fbf4a30, self=0x0000000135e43800, args=JSJitSetterCallArgs at 0x00007fff5fbf4a28) + 186 at HTMLMediaElementBinding.cpp:480
    frame #5: 0x00000001060d4a82 XUL`mozilla::dom::GenericBindingSetter(cx=0x00000001303de850, argc=1, vp=0x00007fff5fbf5598) + 706 at BindingUtils.cpp:2526
    frame #6: 0x00000001094e75c8 XUL`js::CallJSNative(cx=0x00000001303de850, native=0x00000001060d47c0, args=0x00007fff5fbf5430)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 184 at jscntxtinlines.h:235
    frame #7: 0x0000000109475c6e XUL`js::Invoke(cx=0x00000001303de850, args=CallArgs at 0x00007fff5fbf5430, construct=NO_CONSTRUCT) + 1358 at Interpreter.cpp:709
    frame #8: 0x000000010945b9e6 XUL`js::Invoke(cx=0x00000001303de850, thisv=0x00007fff5fbf5c78, fval=0x00007fff5fbf5680, argc=1, argv=0x00007fff5fbf5ae0, rval=JS::MutableHandleValue at 0x00007fff5fbf5530) + 902 at Interpreter.cpp:766
    frame #9: 0x000000010949da58 XUL`js::InvokeSetter(cx=0x00000001303de850, thisv=0x00007fff5fbf5c78, fval=Value at 0x00007fff5fbf5680, v=JS::HandleValue at 0x00007fff5fbf5678) + 232 at Interpreter.cpp:851
    frame #10: 0x000000010958e36a XUL`SetExistingProperty(cx=0x00000001303de850, obj=js::HandleNativeObject at 0x00007fff5fbf5890, id=JS::HandleId at 0x00007fff5fbf5888, v=JS::HandleValue at 0x00007fff5fbf5880, receiver=JS::HandleValue at 0x00007fff5fbf5878, pobj=js::HandleNativeObject at 0x00007fff5fbf5870, shape=js::HandleShape at 0x00007fff5fbf5868, result=0x00007fff5fbf5c50) + 1914 at NativeObject.cpp:2288
    frame #11: 0x000000010958d866 XUL`js::NativeSetProperty(cx=0x00000001303de850, obj=js::HandleNativeObject at 0x00007fff5fbf5b20, id=JS::HandleId at 0x00007fff5fbf5b18, value=JS::HandleValue at 0x00007fff5fbf5b10, receiver=JS::HandleValue at 0x00007fff5fbf5b08, qualified=Qualified, result=0x00007fff5fbf5c50) + 838 at NativeObject.cpp:2322
    frame #12: 0x000000010954ff73 XUL`js::SetProperty(cx=0x00000001303de850, obj=JS::HandleObject at 0x00007fff5fbf5bc0, id=JS::HandleId at 0x00007fff5fbf5bb8, v=JS::HandleValue at 0x00007fff5fbf5bb0, receiver=JS::HandleValue at 0x00007fff5fbf5ba8, result=0x00007fff5fbf5c50) + 227 at NativeObject.h:1434
    frame #13: 0x00000001094b192b XUL`SetPropertyOperation(cx=0x00000001303de850, op=JSOP_SETPROP, lval=JS::HandleValue at 0x00007fff5fbf5ce0, id=JS::HandleId at 0x00007fff5fbf5cd8, rval=JS::HandleValue at 0x00007fff5fbf5cd0) + 491 at Interpreter.cpp:315
    frame #14: 0x000000010948e6eb XUL`Interpret(cx=0x00000001303de850, state=0x00007fff5fbf8d20) + 47563 at Interpreter.cpp:2767
    frame #15: 0x0000000109482c22 XUL`js::RunScript(cx=0x00000001303de850, state=0x00007fff5fbf8d20) + 706 at Interpreter.cpp:653
    frame #16: 0x0000000109475dac XUL`js::Invoke(cx=0x00000001303de850, args=CallArgs at 0x00007fff5fbf9520, construct=NO_CONSTRUCT) + 1676 at Interpreter.cpp:729
    frame #17: 0x0000000109c0b29a XUL`js::fun_apply(cx=0x00000001303de850, argc=2, vp=0x00007fff5fbfa8f8) + 1706 at jsfun.cpp:1289
    frame #18: 0x00000001094e75c8 XUL`js::CallJSNative(cx=0x00000001303de850, native=0x0000000109c0abf0, args=0x00007fff5fbfa790)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 184 at jscntxtinlines.h:235
    frame #19: 0x0000000109475c6e XUL`js::Invoke(cx=0x00000001303de850, args=CallArgs at 0x00007fff5fbfa790, construct=NO_CONSTRUCT) + 1358 at Interpreter.cpp:709
    frame #20: 0x000000010945b9e6 XUL`js::Invoke(cx=0x00000001303de850, thisv=0x00007fff5fbfab90, fval=0x00007fff5fbfabc0, argc=2, argv=0x00007fff5fbfad28, rval=JS::MutableHandleValue at 0x00007fff5fbfa890) + 902 at Interpreter.cpp:766
    frame #21: 0x00000001097f41a9 XUL`js::jit::DoCallFallback(cx=0x00000001303de850, frame=0x00007fff5fbfadc8, stub_=0x000000012742f890, argc=2, vp=0x00007fff5fbfad18, res=JS::MutableHandleValue at 0x00007fff5fbfac78) + 1849 at BaselineIC.cpp:9855
    frame #22: 0x000000011769320b

When seeking, HTMLMediaElement does media::TimeIntervals seekableIntervals = mDecoder->GetSeekable(); on the main thread, which calls MediaSourceDecoder::GetBuffered().

Everything in there is thread safe provided you hold the decoder's monitor.

GetSeekable should use the buffered mirror no?

surprising that none of the mochitest/webref triggers it. i got it trying this page: http://steamcommunity.com/broadcast/watch/76561198047709840
(Assignee)

Comment 37

4 years ago
wrong bug #
(Assignee)

Updated

4 years ago
Blocks: 1188210
You need to log in before you can comment on or make changes to this bug.