I think this was missed when integrating DefaultMediaSourceFactory with
SingleSampleMediaSource.Factory in
315ba6f324
Issue: #8430
#minor-release
PiperOrigin-RevId: 350759580
There is a race condition when initializing the downloads database. The
constructor of the DownloadManager kicks-off the database initialization
in its internal thread, but at the same time an app can try to access
the database directly through the manager's download index, e.g. doing
DonwloadManager manager = new ...
manager.getDownloadIndex().getDownload("id");
might enter DefaultDownloadIndex.ensureInitialized() from two threads.
When upgrading the downloads table from version 2 to version 3, the
first thread that enters the database transaction in ensureInitialized()
will drop and recreate the table using the v3 schema. Then, the second
thread will attempt to read from the newly created table using the v2
schema, which will fail.
This race condition was not introduced in 2.12 but was there already.
However, prior to 2.12, the code only dropped and re-created and the
table and did not attempt to read any data. Hence, if the race condition
happened, the code would drop and create the table twice, but no error
would occur.
Issue: #8420
#minor-release
PiperOrigin-RevId: 350745463
Without this feature it's impossible to nicely merge multiple sources
with different durations if these durations are not known exactly
before the start of playback.
Issue: #8422
PiperOrigin-RevId: 350567625
This allows to set preferences based on MIME type for video and audio.
The MIME type preference is applied after other explicit preferences
and restrictions (e.g. language or max resolution), but before implicit
preferences like bitrate.
Issue: #8320
PiperOrigin-RevId: 350550543
Add assertion to check an output format has been propagated before
returning an output buffer when operating MediaCodec in asynchronous
mode.
PiperOrigin-RevId: 350534918
`stop(true)` is almost the same as `clearMediaItems(); stop();`, except that
any player error isn't cleared. Clearing media items more clearly expresses the
intent.
PiperOrigin-RevId: 350516748
In many cases we just used "playback speed" as a detailed Javadoc
parameter or return type definition. This doesn't define which scale
the speed is using.
PlaybackParameters as the main point to set the speed already uses a
more precise wording to describe the value as a factor by which playback
will be sped up.
This change replaces other usages of "playback speed" with this wording
whereever we would usually add a unit, keeping "playback speed" for
summary statements etc to reference the general concept that doesn't
usually require a unit.
PiperOrigin-RevId: 350379139
The experimental setting shows positive results and can be turned
on by default. To avoid adaptation between HLS audio formats without
bitrates, we need to ensure that only formats with bitrates are
considered for adaptation.
Also added tests for these features.
Issue: #5111
PiperOrigin-RevId: 350315296
The AsynchronousMediaCodecCallback has logic to retain a pending
output format in case flush() is called. This commit fixes a case where
calling flush() again while an output format is pending would nullify
the pending output format.
A unit test is added in AsynchronousMediaCodecCallback but not the
AsynchronousMediaCodecAdapter. That is because the adapter operates
directly on top of MediaCodec, but Robolectric's ShadowMediaCodec
produces an output format on every MediaCodec.start(). This is
unrealistic when operating MediaCodec in asynchronous mode where we
need to call MediaCodec.start() after every MediaCodec.flush().
PiperOrigin-RevId: 350176659
- The AdaptiveTrackSelection doesn't need to use the experimental
terminolgy because the code is always triggered if there are multiple
adaptive selections.
- It's also confusing to pass the state on the outside after the object
creation, so moving everything into a simple control flow again where
the adaptation checkpoints are passed in via the constructor.
- Instead of triple arrays, we can use more readable named structures.
- The calculation of the checkpoints can be cleaned up to be more
readable by moving things to helper methods.
- The reserved bandwidth from all fixed track selections is really just
a special case of multiple parallel adaptataions. So this logic doesn't
need to be separate.
- The whole logic also didn't have test coverage so far. Added tests
for the actual adaptation using these checkpoints and the builder
calculating the checkpoints.
Overall this should be a no-op change.
PiperOrigin-RevId: 350162834
In this change -
Handling the sequence number discontinuity in caption channel packet header.
The processCurrentPacket returns if the packet length does not match with the currentIndex. That assumption is wrong. As per spec the the packet can end on reception of next cc_type = 0x3.
if responseCode and responseMessage ara available always throws an InvalidResponseCodeException instead of HttpDataSourceException, so in onPlayerError method the http status code and message can be used to decide what will be the next step.
*** Original commit ***
DataSource.open() throws if already opened.
Update DataSource implementations to throw an error if open() is called
when the DataSource is already open.
***
PiperOrigin-RevId: 348783425
Estimating the playback frame-rate, querying the display refresh rate, and
setting the surface frame-rate, are all closely related to one another. In
particular because setting the surface frame-rate can directly cause the
display refresh rate to change. It therefore makes sense to move surface
frame-rate adjustment into the helper.
This also makes it easier to re-use the logic in other video renderers.
PiperOrigin-RevId: 348455864
This replaces all the duplicated logic previously implemented in
FakeSampleStream and more closely follows the pattern of how
SampleStreams are used from real MediaPeriods.
Some tests needed adjustments because using real the SampleQueue
improved behaviour:
- Waiting for isLoading is only needed once even across period
boundaries because the real SampleQueue doesn't have the on/off
pattern.
- AnalyticsCollectorTest.playlistOperations() was wrongly asserting
that some pre-buffering events. The new version is more intuitively
correct we pre-buffer the second item during the initial loading
phase (thus period1seq1) and keep the buffer in the queue after
the removal operation.
PiperOrigin-RevId: 348440255
I decided not to migrate all the tests in one CL to keep the diff
manageable. I'll make follow-up CLs to migrate the tests, and eventually
delete TeeCodec and all associated logic.
I couldn't completely remove the dump diff because
ShadowMediaCodec.getCodecInfo() (which would give me access to the MIME
type) doesn't seem to work properly - it returned video/avc when
name=exotest.audio.aac, and looking into the code it looks like there's
some native methods that are missing shadow implementations.
PiperOrigin-RevId: 347991956
The adaptive period currently extends the base (non-adaptive) period
to share common MediaPeriod boilerplate code.
However, once we start using the real SampleQueue in FakeMediaPeriod
the common code becomes even less and the overhead to support
multiple stream implementation from the base class is no longer
worth it. Thus, this change removes the class hierarchy and copies
the common parts to FakeAdaptiveMediaPeriod.
PiperOrigin-RevId: 347990468
This was added due to a misunderstanding - we're more interested in
testing the edge case of trying to read the last zero bytes of a
non-zero-byte resource.
In a future change I want to be able test reading a subrange, so each
TestResource will need to be at least N bytes long.
PiperOrigin-RevId: 347980843
PlaybackStatsListener has a method whose original intention was to be
called when the player is releaed to finish all pending sessions.
However, this also meant that later events (e.g. onVideoDecoderDisabled)
could create new sessions because the old one was already finished.
Use the new onPlayerReleased callback to implement this properly and to
fix the unintentional new session creation.
PiperOrigin-RevId: 347809527
Previously the `AdTagLoader` only had one listener which meant that updates
that should affect all periods with matching identifiers in the timeline only
affected the last-attached one. Fix this by having `AdTagLoader` track all its
listeners.
Issue: #3750
PiperOrigin-RevId: 347571323
Currently we don't remove the AnalyticsListeners registed to
SimpleExoPlayer after calling release. We didn't do this mainly
because there are messages triggered as part of the release that
still cause interesting events (e.g. decoderDisabled with the
final counters, final dropped counts etc).
However, we should fully release/remove the listeners once these
pending events are delivered to:
1. Not leak listener implementations (e.g. if the listener is an
Activity)
2. Ensure we don't send future events that may cause listeners
to unintentionally access released or nulled variables. This
could happen for example if someone calls a player method
after the player was released.
In addition, we can add a onPlayerReleased callback to
AnalyticsListener to allow implementations to clean themselves up
once all pending events are delivered.
PiperOrigin-RevId: 347434344
Fixed the verticalAnchorType and horizontalAnchorType calculation
The anchorID 0, 1 and 2 should correspond to verticalAnchorType=ANCHOR_TYPE_START, anchorID 3, 4, 5 is ANCHOR_TYPE_MIDDLE, anchorID 6, 7 and 8 is ANCHOR_TYPE_END
The anchorID 0, 3 and 6 should correspond to horizzonatlAnchor=ANCHOR_TYPE_START, anchorID 1, 4, 7 is ANCHOR_TYPE_MIDDLE, anchorID 2, 5 and 8 is ANCHOR_TYPE_END
When a listener is removed or released we may not have called
onEvents for events that happened before this point. To ensure
listeners don't miss events we need to trigger a final onEvents
with all events we have happened so far (if any).
PiperOrigin-RevId: 346553030
This will allow Player to move in common without
trackSelector and all its many dependency.
Currently all users of `getTrackSelector` are
downcasting it in `DefaultTrackSelector`, this change
thus does not break them.
Track selection API is intended to be reworked, methods
will be added to the currently empty interface.
#player-to-common
PiperOrigin-RevId: 346159765
Previously the helper would constantly lose (or never establish) sync
for non-1x playback speeds. This changes the helper to account for
other playback speeds correctly.
PiperOrigin-RevId: 345458859
This was reported for SSA/ASS in PR #8265, but it seems to me the
SubRip part of the Matroska spec is similarly loose, so this change
handles null-terminated strings in both.
#minor-release
PiperOrigin-RevId: 345452667
The old tag reflects the original name of this class. It was renamed in
2017:
<unknown commit>
Possibly the old name was kept for compatibility with existing logs
analysis? I didn't see an explicit discussion of this in the review
comments of the rename change.
The current tag confused me slightly - I assumed a line was being
emitted by the android.media.AudioTrack rather than ExoPlayer's
DefaultAudioSink.
PiperOrigin-RevId: 345450056
The `AudioProcessor` interface requires that no more input is queued after
queueing end of stream, but `DefaultAudioSink` did queue more input and the
implementation of `SonicAudioProcessor` actually relied on this to drain output
at the end of the stream.
Fix this behavior by getting `Sonic` output in `getOutput` and having
`DefaultAudioSink` only queue input to processors that are not draining.
Also add TODOs to clean up audio processor implementations where the code
handles interaction that doesn't conform to the interface.
PiperOrigin-RevId: 345406478
As Player depends on VideoScalingMode, and
Renderer should not move to common,
to move Player to common, VideoScalingMode
needs to move first.
#player-to-common
PiperOrigin-RevId: 345314448
Changes MetadataRetriever and Transformer so that their
respective tests don't need to manually control the SystemClock
in order to execute taks posted with delay from Loader.
PiperOrigin-RevId: 345024140
Previously `MediaPeriodQueue` would return null if an ad media URI hadn't
loaded yet, but this meant that the player could be stuck in `STATE_READY` if
an `AdsLoader` unexpectedly didn't provide an ad URI. Fix this behavior by
masking ad media periods. `MaskingMediaPeriod` no longer requires a
`MediaSource` to instantiate it.
This also fixes a specific case where playback gets stuck when using the IMA
extension with an empty ad where the IMA SDK unexpectedly doesn't notify the ad
group fetch error.
Issue: #8205
PiperOrigin-RevId: 344984824
This method should be assumed to clear the data of the underlying array
(it will do this if the new limit > data.length).
This means it should only be called (directly) before writing into the
backing array.
It shouldn't be used as a shorthand for position=0, limit=x - those
should be two explicit method calls.
Most of these changes are no-ops, but they make the code more correct.
The TS SectionReader can't be easily changed to be 'safe', because it
relies on sectionData maintaining state between iterations of the while
loop. Instead I've added comments justifying the existing code.
PiperOrigin-RevId: 344515340
This ensures we have full test coverage for proguard configs now.
The only configs not covered by tests are:
- IMA and OkHttp which copy recommended configs from the respective
library. I couldn't reproduce failures by removing them (and thus
couldn't write a test that ensures they are correct).
- Some dontwarn lines that just suppress warnings.
In addition, this change fixes a couple of related issues:
- Moved AV1 proguard config to correct module.
- Removed mentioning of deprecated ExtractorMediaSource from README
- Suppressed warning from IMA code that prevent proguarding under
strict rules
- Fixed wrong proguard exclusion in VP9 module.
- Moved FLAC exclusion (DefaultExtractorsFactory) to correct module.
- Added AlertDialog suppression for de-jetified code.
- Removed unusued dependency from UI module that causes large APK
size increase.
#exofixit
#minor-release
PiperOrigin-RevId: 344427532
Previously the PlaybackStatsListener needed to handle all events
individually, which required to keep some state of the player and
to resolve potentially transient state changes.
Using onEvents allows to channel all simultanous updates through
one method so that no transient player state and other
inconsistencies need to be handled. This makes the logic easier
to read.
In addition it also allows to resolve all simultaneous events to
use one EventTime (with one timestamp).
#exofixit
PiperOrigin-RevId: 344415459
In many cases it doesn't matter for the test itself how many windows
a timeline has, or even how the timeline of a MediaSource looks like.
And since we introduced the MediaItem-based APIs, single-window
Timelines are the only fully supported Timelines. Thus there is no
point in specifiying this explicitly.
Using these assumptions, we can remove some boilerplate when setting
up standard FakeTimelines or FakeMediaSources with a standard
FakeTimeline.
#exofixit
PiperOrigin-RevId: 344210395
The current code creates placeholder metadata elements if there is no
static metadata. This causes onStaticMetadataChanged callbacks even
if there is no metadata.
Instead, we can keep the empty list as the static metadata is already
documented to be an empty list if the metadata is unavailable.
#exofixit
PiperOrigin-RevId: 344071639
- Remove restriction on `AdsMediaSource`s in playlists in `ExoPlayerImpl`.
- Allow playing playlists of `AdsMediaSource`s in the demo app.
- Add a sample with ads in a playlist in the demo app.
Issue: #3750
PiperOrigin-RevId: 344018774
*** Original commit ***
Suppress warnings in preparation for Checker Framework 3.7.1 upgrade.
LSC: go/checker-lsc
Tested:
TAP train for global presubmit queue
http://test/OCL:342788973:BASE:342817196:1605636478036:6c558c0c
***
PiperOrigin-RevId: 343895651
The EventTime wasn't part of the onEvents callbacks so far because the
individual events may have different event times (e.g. if they relate to
different media periods). By adding a query method to obtain the
EventTime by event type, we can solve this issue.
#exofixit
PiperOrigin-RevId: 343818819
Mentioning the realtionship on every other callback makes
it easier for the casual developer to understand the
relationship between the methods and discover the existence
of onEvents.
PiperOrigin-RevId: 343811032
Previously, VideoFrameReleaseTimeHelper didn't receive PTS values for
frames that were skipped. This would lead to unnecessary sync loss when
encountering such buffers, because the helper would see a frame-rate
change (i.e., the next frame being significantly longer than previous
ones). After this change, VideoFrameReleaseTimeHelper is notified of
all frame PTS values, so it can retain sync in this case.
This change also propagates onStarted and onPositionReset to the helper.
This paves the way for decoupling frame-rate estimation from release
adjustment sync. Note that currently, loss of sync effectively resets
both. This is unnecessary. For example, if the renderer is paused and
later resumed, then release adjustment sync needs to be reset, but any
frame-rate estimate we have is still valid.
PiperOrigin-RevId: 343803531
RunnableFutureTask is not reusable. Trying to reuse it meant that a
failure in one doWork() call would cause subsequent download() calls
to (a) not block until the runnable has finished executing (does not
apply when using a direct executor), and (b) throw the same failure
as thrown from the first doWork() call.
This could cause #8078 if the initial failure occurred before the
content length was resolved. Retries are not blocked on their work
completing due to (a), and the download would be marked as failed due
to (b). The work itself could then resolve the content length, which
causes the stack trace in this issue.
Issue: #8078
PiperOrigin-RevId: 343498252
This callback allows listeners to know when all simultanous changes
have been handled and the values reported through callbacks are
again completely consistent with value obtained from Player
getter calls.
PiperOrigin-RevId: 343476639
When a stream has duplicate timestamps we currently discard to
the last sample with the specified discardTo timestamp, but
it should be the first one to adhere to the method doc and the
intended usage.
#minor-release
PiperOrigin-RevId: 343458870
Background:
1. When the player has multiple audio renderers, by default they share a
single AudioSink.
2. When any new renderer is enabled, all disabled renderers are reset
prior to the new renderer being enabled. This is to give them a chance
to free up resources in case the renderer being enabled needs them. These
reset calls are expected to be no-ops for renderers that have never been
enabled.
The issue:
The problematic case arises when there are two audio renderers and a third
renderer (e.g., text) is being enabled. In this case, the disabled audio
renderer's reset call ends up resetting the AudioSink that's shared with the
enabled audio renderer. The enabled audio renderer is then unable to make
progress, causing playback to freeze.
This is a minimal fix that directly prevents the mentioned issue. There are
multiple follow-ups that would probably make sense:
1. Having ExoPlayerImplInternal track which renderers need to be reset, and
only resetting those renderers rather than all that are disabled. This
seems like a good thing to do regardless, rather than relying on those
calls being no-ops.
2. If we want to continue sharing AudioSink, we need to formalize this much
better and make sure we have good test coverage. Messages like
MSG_SET_VOLUME are also delivered to the AudioSink multiple times via
each of the renderers, which works currently because DefaultAudioSink
no-ops all but the first call in each case. This is pretty fragile though!
Issue: #8203
#minor-release
PiperOrigin-RevId: 343296081
*** Original commit ***
Suppress warnings in preparation for Checker Framework 3.7.1 upgrade.
LSC: go/checker-lsc
Tested:
tap_presubmit: http://test/OCL:342788975:BASE:342885903:1605667635149:7731b723
Some tests failed; test failures are believed to be unrelated to this CL
***
PiperOrigin-RevId: 343251530
This only has a couple of simple tests for now. We'll add more tests
after we've written some concrete sub-class tests for various
DataSource implementations.
I've included a concrete FileDataSourceContractTest as a demonstration.
PiperOrigin-RevId: 342851187
This ensures the buffer is not full when the `DefaultLoadControl` determines
whether we should continue loading and thus prevents a false warning about
not having enough memory left.
PiperOrigin-RevId: 342616623
