Closed
Bug 1228939
Opened 8 years ago
Closed 8 years ago
Remove MediaDecoderReader::IsMediaSeekable()
Categories
(Core :: Audio/Video: Playback, defect, P2)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: jwwang, Assigned: jwwang)
Details
Attachments
(2 files)
We will use MediaEventSource to notify the decoder that this media is not seekable. This eliminate the need for cross-thread reading from members of the readers and make a more consistent thread-model for the readers.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jwwang
Updated•8 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•8 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=43360c567566
Assignee | ||
Comment 2•8 years ago
|
||
Bug 1228939. Part 1 - use MediaEventSource to notify the decoder when the media is not seekable. r=jya.
Attachment #8694095 -
Flags: review?(jyavenard)
Assignee | ||
Comment 3•8 years ago
|
||
Bug 1228939. Part 2 - remove unused code. r=jya.
Attachment #8694097 -
Flags: review?(jyavenard)
Assignee | ||
Comment 4•8 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a12fa5dcc81
Assignee | ||
Updated•8 years ago
|
Attachment #8694095 -
Flags: review?(jyavenard)
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8694095 [details] MozReview Request: Bug 1228939. Part 1 - 1. add mSeekable to MediaInfo. 2. use MediaEventSource to notify the decoder when the media is not seekable. r=jya. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26657/diff/1-2/
Attachment #8694095 -
Attachment description: MozReview Request: Bug 1228939. Part 1 - use MediaEventSource to notify the decoder when the media is not seekable. r=jya. → MozReview Request: Bug 1228939. Part 1 - 1. add mSeekable to MediaInfo. 2. use MediaEventSource to notify the decoder when the media is not seekable. r=jya.
Attachment #8694095 -
Flags: review?(jyavenard)
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8694097 [details] MozReview Request: Bug 1228939. Part 2 - remove unused code. r=jya. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26659/diff/1-2/
Assignee | ||
Comment 7•8 years ago
|
||
Per discussion, add mMediaSeekable to MediaInfo to indicate if this media is seekable or not when reading metadata.
Comment 8•8 years ago
|
||
Comment on attachment 8694095 [details] MozReview Request: Bug 1228939. Part 1 - 1. add mSeekable to MediaInfo. 2. use MediaEventSource to notify the decoder when the media is not seekable. r=jya. https://reviewboard.mozilla.org/r/26657/#review24241 ::: dom/media/MediaInfo.h:404 (Diff revision 2) > + bool mMediaSeekable = true; personally I like this being done in the constructor. not a fan of C++11 inline member initialisation
Attachment #8694095 -
Flags: review?(jyavenard) → review+
Comment 9•8 years ago
|
||
Comment on attachment 8694097 [details] MozReview Request: Bug 1228939. Part 2 - remove unused code. r=jya. https://reviewboard.mozilla.org/r/26659/#review24243 I would have merged part 1 and part2 as part 1 includes piece of code that is removed from part2 and you get to wonder where the logic come from. would be more apparent in a combined patch
Attachment #8694097 -
Flags: review?(jyavenard) → review+
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #8) > Comment on attachment 8694095 [details] > MozReview Request: Bug 1228939. Part 1 - 1. add mSeekable to MediaInfo. 2. > use MediaEventSource to notify the decoder when the media is not seekable. > r=jya. > > https://reviewboard.mozilla.org/r/26657/#review24241 > > ::: dom/media/MediaInfo.h:404 > (Diff revision 2) > > + bool mMediaSeekable = true; > > personally I like this being done in the constructor. not a fan of C++11 > inline member initialisation It comes in handy when every constructor initializes the member with the same value or when there is no constructor at all which is the case of MediaInfo.
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #9) > Comment on attachment 8694097 [details] > MozReview Request: Bug 1228939. Part 2 - remove unused code. r=jya. > > https://reviewboard.mozilla.org/r/26659/#review24243 > > I would have merged part 1 and part2 as part 1 includes piece of code that > is removed from part2 and you get to wonder where the logic come from. would > be more apparent in a combined patch Which portion of part2 do you mean that can be moved to part1? part2 is all about removing code.
Comment 12•8 years ago
|
||
(In reply to JW Wang [:jwwang] from comment #10) > It comes in handy when every constructor initializes the member with the > same value or when there is no constructor at all which is the case of > MediaInfo. You can use delegated constructor now https://developer.mozilla.org/en-US/docs/Using_CXX_in_Mozilla_code so you don't have to initialize the value in each constructor. only one
Comment 13•8 years ago
|
||
(In reply to JW Wang [:jwwang] from comment #11) > Which portion of part2 do you mean that can be moved to part1? part2 is all > about removing code. merge both patches into 1.
Assignee | ||
Comment 14•8 years ago
|
||
(In reply to Jean-Yves Avenard [:jya] from comment #13) > merge both patches into 1. will do.
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/19c212559258
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
You need to log in
before you can comment on or make changes to this bug.
Description
•