Closed
Bug 1272232
Opened 9 years ago
Closed 9 years ago
Remove use of FlushableTaskQueue::Flush() from AppleVDADecoder::Flush()
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla49
| Tracking | Status | |
|---|---|---|
| firefox49 | --- | fixed |
People
(Reporter: jwwang, Assigned: jwwang)
References
Details
Attachments
(5 files)
Similar to bug 1272225.
| Assignee | ||
Comment 1•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52644/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52644/
Attachment #8752566 -
Flags: review?(jyavenard)
Attachment #8752567 -
Flags: review?(jyavenard)
Attachment #8752568 -
Flags: review?(jyavenard)
Attachment #8752569 -
Flags: review?(jyavenard)
Attachment #8752570 -
Flags: review?(jyavenard)
| Assignee | ||
Comment 2•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52646/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52646/
| Assignee | ||
Comment 3•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52648/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52648/
| Assignee | ||
Comment 4•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52650/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52650/
| Assignee | ||
Comment 5•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52652/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/52652/
Comment 6•9 years ago
|
||
Comment on attachment 8752566 [details]
MozReview Request: Bug 1272232. Part 1 - move code around so we can extract common code in P2. r=jya.
https://reviewboard.mozilla.org/r/52644/#review49838
Attachment #8752566 -
Flags: review?(jyavenard) → review+
Updated•9 years ago
|
Attachment #8752567 -
Flags: review?(jyavenard) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8752567 [details]
MozReview Request: Bug 1272232. Part 2 - extract common code to the parent class. r=jya.
https://reviewboard.mozilla.org/r/52646/#review49840
Comment 8•9 years ago
|
||
Comment on attachment 8752568 [details]
MozReview Request: Bug 1272232. Part 3 - constify some members and make them private when possible. r=jya.
https://reviewboard.mozilla.org/r/52648/#review49842
::: dom/media/platforms/apple/AppleVDADecoder.h:91
(Diff revision 1)
> - // OutputFrame is thread-safe.
> - nsresult OutputFrame(CVPixelBufferRef aImage,
> - AppleFrameRef aFrameRef);
> -
> protected:
> + AppleVDADecoder(const VideoInfo& aConfig,
How can you make the constructor private? How would the Apple PDM create it unless you play with friendship.
can't make that protected.
::: dom/media/platforms/apple/AppleVDADecoder.cpp:37
(Diff revision 1)
> #define LOG(...) MOZ_LOG(GetPDMLog(), mozilla::LogLevel::Debug, (__VA_ARGS__))
> //#define LOG_MEDIA_SHA1
>
> namespace mozilla {
>
> +static uint32_t ComputeMaxRefFrames(const mozilla::MediaByteBuffer* aExtraData)
WE'RE already in the Mozilla namespace, you can drop it from MediaByteBuffer.
Attachment #8752568 -
Flags: review?(jyavenard) → review+
Comment 9•9 years ago
|
||
Comment on attachment 8752569 [details]
MozReview Request: Bug 1272232. Part 4 - remove use of FlushableTaskQueue::Flush(). r=jya.
https://reviewboard.mozilla.org/r/52650/#review49844
Attachment #8752569 -
Flags: review?(jyavenard) → review+
Comment 10•9 years ago
|
||
Comment on attachment 8752570 [details]
MozReview Request: Bug 1272232. Part 5 - remove use of FlushableTaskQueue. r=jya.
https://reviewboard.mozilla.org/r/52652/#review49846
Attachment #8752570 -
Flags: review?(jyavenard) → review+
Comment 11•9 years ago
|
||
https://reviewboard.mozilla.org/r/52648/#review49842
> How can you make the constructor private? How would the Apple PDM create it unless you play with friendship.
>
> can't make that protected.
Had forgotten about CreateVDADecoder
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4fdac9c2c14b
https://hg.mozilla.org/integration/mozilla-inbound/rev/64bf09ce3533
https://hg.mozilla.org/integration/mozilla-inbound/rev/f158ef82936c
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7c63a0d98a2
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bb3981b0c70
Comment 13•9 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/4fdac9c2c14b
https://hg.mozilla.org/mozilla-central/rev/64bf09ce3533
https://hg.mozilla.org/mozilla-central/rev/f158ef82936c
https://hg.mozilla.org/mozilla-central/rev/d7c63a0d98a2
https://hg.mozilla.org/mozilla-central/rev/5bb3981b0c70
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•