New issue
Advanced search Search tips

Issue 769434 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 718641



Sign in to add a comment

Clean up ByPts warning of track buffer gap

Project Member Reported by wolenetz@chromium.org, Sep 27 2017

Issue description

In core PTS/DTS cleanup CL (https://chromium-review.googlesource.com/c/chromium/src/+/678443, currently WIP),     WarnIfTrackBufferExhaustionSkipsForward() can warn unnecessarily on some kinds of overlap when buffering ByPts:

In out-of-order stream track_buffer_, IPBBBBB in decode order (IBBBBBP in PTS order), if the last track_buffer_ GOP actually is adjacent to the next non track_buffer_ keyframe in PTS sequence (but not in DTS sequence), the warning of jumping forward can occur unnecessarily (eg last track_buffer_ B-frame is twice the normal frame duration away from next GOP due to P-frame intervening in PTS order).

This bug tracks removing unnecessary instances of this warning (perhaps by tracking the track_buffer_'s most recent GOP's highest-output PTS, not last-output PTS for comparison to next non-track-buffer keyframe PTS).
 
Blockedon: 718641
Status: Started (was: Assigned)
Hmm. It looks like I may need to fix this as part of the core PTS/DTS cleanup CL, since the same problem results in playback stall in transition out of track_buffer to next range (e.g. SBSTest.Middle_Overlap_Selected_4)
Blockedon: -718641
Blocking: 718641
https://chromium-review.googlesource.com/c/chromium/src/+/678443 patch set 27 contains the fix for this.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f

commit 8efe7217bc8d16be91cc94b0de80f6a5bd5d704f
Author: Matt Wolenetz <wolenetz@chromium.org>
Date: Tue Oct 03 20:40:26 2017

MSE: Buffer media by PTS when kMseBufferByPts feature is enabled

Other than mechanical* DTS/PTS pieces, this CL contains the bulk of the PTS/DTS compliance work:
1) SBS<SBRByPts> and SBRByPts order buffers within GOP by DTS, between GOP by PTS.
   See especially SBRByPts::GetBufferIndexAt(...) and ::CanAppendBuffersToEnd(...)
2) SBS now has highest_timestamp_in_append_sequence_ and
   highest_buffered_end_time_in_append_sequence_, along with updated PrepareRangesForNextAppend(),
   to correctly *not* remove self-overlapping out-of-order buffers in a sequence of appends within
   the same coded frame group when buffering ByPts. Similarly, usage of and implementation of GC
   helper FreeBuffersAfterLastAppended() consults these to not unduly evict pieces of an
   out-of-order GOP that is before media_time.
3) SBS now has highest_output_buffer_timestamp_ to help prevent some undue stalling in cases
   like track buffer exhaustion with out-of-order track buffers when buffering ByPts.
4) FP signals SBS of coded frame group starts, including PTS information. SBS decides which to use
   based on ByDts vs ByPts. Also, FP handles re-signalling CFG start time if the start has moved
   earlier across tracks in a muxed SourceBuffer (similar to existing logic for DTS and muxed
   sequence mode). SAP-Type-2 is not explicitly rejected; related FP*OOO* unit tests demonstrate
   current buffering results ByDts vs ByPts for one simple SAP-Type-2 scenario.

Many related unit tests are updated. Notably, SBSTests already had out-of-order buffering
in many of its cases.

* Much of the mechanical stuff is to make SBRBy{Pts,Dts} use the intended time type internally,
  while letting SBS get away with using DTS still in many places where eventually it'll use PTS
  when SBRByDts gets dropped. See the set of new templated SBS::Range*(...) adapter methods.

BUG= 718641 , 769434 , 402502 , 398130 

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ib2abb816d240a6f74362bb83061ac4157dab210b
Reviewed-on: https://chromium-review.googlesource.com/678443
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506161}
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/chunk_demuxer.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/chunk_demuxer.h
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/frame_processor.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/frame_processor.h
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/frame_processor_unittest.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_range.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_range.h
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_range_by_dts.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_range_by_dts.h
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_range_by_pts.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_range_by_pts.h
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_stream.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_stream.h
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/filters/source_buffer_stream_unittest.cc
[modify] https://crrev.com/8efe7217bc8d16be91cc94b0de80f6a5bd5d704f/media/test/pipeline_integration_test.cc

Status: Fixed (was: Started)
#4 fixes this.

Sign in to add a comment