Reduce ref-counting overhead of MediaFormatReader::HandleDemuxedSamples()

NEW
Assigned to

Status

()

Core
Audio/Video: Playback
P3
normal
9 months ago
9 months ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

9 months ago
http://searchfox.org/mozilla-central/rev/224cc663d54085994a4871ef464b7662e0721e83/dom/media/MediaFormatReader.cpp#2037

We can reduce one AddRef/Release pair by transferring the ownership of |sample| for it is no longer used and will be removed by |decoder.mQueuedSamples.RemoveElementAt(0)| soon.
(Assignee)

Updated

9 months ago
Assignee: nobody → jwwang
Priority: -- → P3
Comment hidden (mozreview-request)
(Assignee)

Updated

9 months ago
Attachment #8865766 - Flags: review?(jyavenard)
Attachment #8865766 - Flags: review?(gsquelart)
(Assignee)

Comment 2

9 months ago
mozreview-review
Comment on attachment 8865766 [details]
Bug 1363283 - Reduce ref-counting overhead of MediaFormatReader::HandleDemuxedSamples().

https://reviewboard.mozilla.org/r/137386/#review140446

::: dom/media/Benchmark.cpp:308
(Diff revision 1)
>    MOZ_ASSERT(OnThread());
>    if (mFinished || mSampleIndex >= mSamples.Length()) {
>      return;
>    }
>    RefPtr<Benchmark> ref(mMainThreadState);
> -  mDecoder->Decode(mSamples[mSampleIndex])
> +  mDecoder->Decode(mSamples[mSampleIndex].forget())

Hi Gerald,
Can you review the change in Benchmark.cpp to make sure it is safe to transfer the ownership of mSamples[mSampleIndex]?

Comment 3

9 months ago
mozreview-review
Comment on attachment 8865766 [details]
Bug 1363283 - Reduce ref-counting overhead of MediaFormatReader::HandleDemuxedSamples().

https://reviewboard.mozilla.org/r/137386/#review140886

I don't like it.

It makes the code much more painful to read, more complicated to maintain for no proven benefit
Attachment #8865766 - Flags: review?(jyavenard)

Comment 4

9 months ago
mozreview-review
Comment on attachment 8865766 [details]
Bug 1363283 - Reduce ref-counting overhead of MediaFormatReader::HandleDemuxedSamples().

https://reviewboard.mozilla.org/r/137386/#review140888

I should add, that the aim was to have a shared MediaRawData between the MSE demuxer (that stores all those raw data in an array) and the MediaDataDecoder.

This would no longer be possible with this.

Comment 5

9 months ago
mozreview-review
Comment on attachment 8865766 [details]
Bug 1363283 - Reduce ref-counting overhead of MediaFormatReader::HandleDemuxedSamples().

https://reviewboard.mozilla.org/r/137386/#review140934

(Deferring to jya's review)
Attachment #8865766 - Flags: review?(gsquelart)
You need to log in before you can comment on or make changes to this bug.