Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2064073004:
MSE: Update FrameProcessor comments w.r.t. current spec (Closed)
|
Created:
4 years, 6 months ago by wolenetz Modified:
4 years, 6 months ago Reviewers:
chcunningham CC:
chromium-reviews, posciak+watch_chromium.org, feature-media-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMSE: Update FrameProcessor comments w.r.t. current spec
The spec's coded frame processing algorithm steps have changed slightly.
This change updates the related code comments to link to the June 9,
2016 editor's draft, adds a TODO referencing bug 398141, and removes a
TODO referencing bug 351166.
BUG=398141, 351166
R=chcunningham@chromium.org
Committed: https://crrev.com/8b69a4ca4fb53925d723fe517a637b0d88736582
Cr-Commit-Position: refs/heads/master@{#399969}
Patch Set 1 #
Total comments: 4
Messages
Total messages: 18 (6 generated)
The CQ bit was checked by wolenetz@chromium.org to run a CQ dry run
Please take a look. Thanks!
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2064073004/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios-device on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL) ios-device-gn on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL)
The CQ bit was checked by wolenetz@chromium.org to run a CQ dry run
On 2016/06/14 23:49:21, commit-bot: I haz the power wrote: > Dry run: Try jobs failed on following builders: > ios-device on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL) > ios-device-gn on tryserver.chromium.mac (JOB_TIMED_OUT, no build URL) Friendly ping :)
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2064073004/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... File media/filters/frame_processor.cc (left): https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... media/filters/frame_processor.cc:213: // https://dvcs.w3.org/hg/html-media/raw-file/d471a4412040/media-source/ I noticed these old links are completely busted. Do you think the new links will be more permanently-working? https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... File media/filters/frame_processor.cc (right): https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... media/filters/frame_processor.cc:465: // Otherwise case: (See MediaSourceState's |auto_update_timestamp_offset_|, Am I right that "generate timestamps" case is handled by the mpeg parsers?
Thanks for the review. https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... File media/filters/frame_processor.cc (left): https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... media/filters/frame_processor.cc:213: // https://dvcs.w3.org/hg/html-media/raw-file/d471a4412040/media-source/ On 2016/06/15 18:26:21, chcunningham wrote: > I noticed these old links are completely busted. Do you think the new links will > be more permanently-working? They're better. Permanent they may not be (depends on github). Regardless, eventual PR/REC publication of the spec will yield a more stable permanent snapshot URL even than this one. https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... File media/filters/frame_processor.cc (right): https://codereview.chromium.org/2064073004/diff/1/media/filters/frame_process... media/filters/frame_processor.cc:465: // Otherwise case: (See MediaSourceState's |auto_update_timestamp_offset_|, On 2016/06/15 18:26:21, chcunningham wrote: > Am I right that "generate timestamps" case is handled by the mpeg parsers? Correct (in conjunction with MediaSourceState logic around this). It's not fully compliant at the moment. See https://bugs.chromium.org/p/chromium/issues/detail?id=607372 tracking cleaning that overall behavior.
The CQ bit was checked by wolenetz@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2064073004/1
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
CQ bit was unchecked
Message was sent while issue was closed.
Description was changed from ========== MSE: Update FrameProcessor comments w.r.t. current spec The spec's coded frame processing algorithm steps have changed slightly. This change updates the related code comments to link to the June 9, 2016 editor's draft, adds a TODO referencing bug 398141, and removes a TODO referencing bug 351166. BUG=398141,351166 R=chcunningham@chromium.org ========== to ========== MSE: Update FrameProcessor comments w.r.t. current spec The spec's coded frame processing algorithm steps have changed slightly. This change updates the related code comments to link to the June 9, 2016 editor's draft, adds a TODO referencing bug 398141, and removes a TODO referencing bug 351166. BUG=398141,351166 R=chcunningham@chromium.org Committed: https://crrev.com/8b69a4ca4fb53925d723fe517a637b0d88736582 Cr-Commit-Position: refs/heads/master@{#399969} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/8b69a4ca4fb53925d723fe517a637b0d88736582 Cr-Commit-Position: refs/heads/master@{#399969} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
