Closed Bug 1323931 Opened 5 years ago Closed 5 years ago

Some AccurateSeekingState code cleanup

Categories

(Core :: Audio/Video: Playback, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(6 files, 4 obsolete files)

58 bytes, text/x-review-board-request
kaku
: review+
Details
58 bytes, text/x-review-board-request
kaku
: review+
Details
58 bytes, text/x-review-board-request
kaku
: review+
Details
58 bytes, text/x-review-board-request
kaku
: review+
Details
58 bytes, text/x-review-board-request
kaku
: review+
Details
58 bytes, text/x-review-board-request
kaku
: review+
Details
No description provided.
Assignee: nobody → jwwang
Priority: -- → P3
Comment on attachment 8819254 [details]
Bug 1323931. Part 1 - remove unnecessary comments and logs.

https://reviewboard.mozilla.org/r/99100/#review99664
Attachment #8819254 - Flags: review?(kaku) → review+
Comment on attachment 8819255 [details]
Bug 1323931. Part 2 - use raw pointers to reduce unnecessary ref-counting.

https://reviewboard.mozilla.org/r/99102/#review99662
Attachment #8819255 - Flags: review?(kaku) → review+
Comment on attachment 8819256 [details]
Bug 1323931. Part 3 - remove mIsAudioQueueFinished/mIsVideoQueueFinished and finish queues in place.

https://reviewboard.mozilla.org/r/99104/#review99666
Attachment #8819256 - Flags: review?(kaku) → review+
Comment on attachment 8819257 [details]
Bug 1323931. Part 4 - no need to update mDecoded{Audio,Video}EndTime in OnSeekTaskResolved().

https://reviewboard.mozilla.org/r/99106/#review99670
Attachment #8819257 - Flags: review?(kaku) → review+
Comment on attachment 8819258 [details]
Bug 1323931. Part 5 - remove mSeekedAudioData and mSeekedVideoData.

https://reviewboard.mozilla.org/r/99108/#review99658

::: dom/media/MediaDecoderStateMachine.cpp:1066
(Diff revision 1)
> +      RefPtr<MediaData> audio = AudioQueue().PeekFront();
> +      RefPtr<MediaData> video = VideoQueue().PeekFront();

Why "PeekFront()"? For video-only fast-seek, there might be some audio samples in the queue which are not decoded in this seeking phase. Or, you just want to update the current time to the begining of audio/video queue which is closet to the seek time?
Attachment #8819258 - Flags: review?(kaku) → review+
Comment on attachment 8819259 [details]
Bug 1323931. Part 6 - remove unused functions.

https://reviewboard.mozilla.org/r/99110/#review99672
Attachment #8819259 - Flags: review?(kaku) → review+
Comment on attachment 8819258 [details]
Bug 1323931. Part 5 - remove mSeekedAudioData and mSeekedVideoData.

https://reviewboard.mozilla.org/r/99108/#review99658

> Why "PeekFront()"? For video-only fast-seek, there might be some audio samples in the queue which are not decoded in this seeking phase. Or, you just want to update the current time to the begining of audio/video queue which is closet to the seek time?

The calculation is wrong for video-only seek but it doesn't matter for we won't call UpdatePlaybackPositionInternal() for video-only seek in SeekCompleted().
Thanks for the  review!
Attachment #8819256 - Attachment is obsolete: true
Attachment #8819257 - Attachment is obsolete: true
Attachment #8819258 - Attachment is obsolete: true
Attachment #8819259 - Attachment is obsolete: true
Comment on attachment 8819724 [details]
Bug 1323931. Part 3 - remove mIsAudioQueueFinished/mIsVideoQueueFinished and finish queues in place.

https://reviewboard.mozilla.org/r/99374/#review99680
Attachment #8819724 - Flags: review?(kaku) → review+
Comment on attachment 8819725 [details]
Bug 1323931. Part 4 - no need to update mDecoded{Audio,Video}EndTime in OnSeekTaskResolved().

https://reviewboard.mozilla.org/r/99376/#review99682
Attachment #8819725 - Flags: review?(kaku) → review+
Comment on attachment 8819726 [details]
Bug 1323931. Part 5 - remove mSeekedAudioData and mSeekedVideoData.

https://reviewboard.mozilla.org/r/99378/#review99684
Attachment #8819726 - Flags: review?(kaku) → review+
Comment on attachment 8819727 [details]
Bug 1323931. Part 6 - remove unused functions.

https://reviewboard.mozilla.org/r/99380/#review99686
Attachment #8819727 - Flags: review?(kaku) → review+
Blocks: 1318610
Depends on: 1322799
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32b11cd092be
Part 1 - remove unnecessary comments and logs. r=kaku
https://hg.mozilla.org/integration/autoland/rev/507cdb36aa9a
Part 2 - use raw pointers to reduce unnecessary ref-counting. r=kaku
https://hg.mozilla.org/integration/autoland/rev/9e5732e33055
Part 3 - remove mIsAudioQueueFinished/mIsVideoQueueFinished and finish queues in place. r=kaku
https://hg.mozilla.org/integration/autoland/rev/a77632f19713
Part 4 - no need to update mDecoded{Audio,Video}EndTime in OnSeekTaskResolved(). r=kaku
https://hg.mozilla.org/integration/autoland/rev/1ec140c036f2
Part 5 - remove mSeekedAudioData and mSeekedVideoData. r=kaku
https://hg.mozilla.org/integration/autoland/rev/72e006de2754
Part 6 - remove unused functions. r=kaku
You need to log in before you can comment on or make changes to this bug.