Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync with ExoPlayer 1.3 release #18

Closed
wants to merge 676 commits into from
Closed

Sync with ExoPlayer 1.3 release #18

wants to merge 676 commits into from

Conversation

jdtremblay
Copy link
Member

This PR brings all commits from the latest release for media3-exoplayer https://github.com/androidx/media.

SheenaChhabra and others added 30 commits December 21, 2023 08:02
With fMP4 implementation there will be two writers `DefaultMp4Writer`
and `FragmentedMp4Writer`.

Changes includes:
1. Make Mp4Writer as an abstract class and keep only common functionality
into it.
2. Create a DefaultMp4Writer which contains existing logic to write MP4.
3. The fMP4 logic needs to access `pending sample buffer info` at various
places, so did refactoring to split List<Pair<BufferInfo, ByteBuffer>>
into two separate lists.

PiperOrigin-RevId: 592861398
This is similar to the HLS fix in androidx@770ca66

Similar to HLS, the original problem here was **not** modifying the
`Format` for caption tracks
embedded into the video stream. I tried just updating the format in
both places, but that caused new failures because the new
('transcoded') format was then fed into `FragmentedMp4Extractor` as
part of `closedCaptionFormats`, which resulted in the CEA-608 data
being emitted from `FragmentedMp4Extractor` with the incorrect
`application/x-media3-cues` MIME type (but the bytes were actually
CEA-608), meaning the transcoding wrapper passed it through without
transcoding and decoding failed (because obviously CEA-608 bytes can't
be decoded by `CueDecoder` which is expecting a `Bundle` from
`CuesWithTiming.toBundle`.

To resolve this we keep track of the 'original' caption formats inside
`TrackGroupInfo`, so we can feed them into `FragmentedMp4Extractor`.
For all other usages in `DashMediaPeriod` we use the 'transcoded'
caption formats.

PiperOrigin-RevId: 592866262
Essentially a manual revert of androidx@ba0724c

PiperOrigin-RevId: 592869049
Replace the event for notifying fallback to cover codec initialization in
general (but keeping the list of errors).

Add a flag to control whether to try non-primary codecs, with the same
documentation as the similar flag in ExoPlayer's renderer.

Make the class final as it shouldn't be necessary to subclass it.

PiperOrigin-RevId: 592869868
PiperOrigin-RevId: 592873355
Instead, for input videos, use the colorInfo provided by the extractor. Similarly, for input images, use sRGB, the only color currently in use.

Textures do still need the input ColorInfo provided though.

PiperOrigin-RevId: 592875967
The previous code led me to misread this stack trace as a null pointer
exception, but it's really an index out of bounds exception:

```
Caused by: androidx.media3.exoplayer.upstream.Loader$UnexpectedLoaderException: Unexpected IllegalArgumentException: null
  at androidx.media3.exoplayer.upstream.Loader$LoadTask.run(Loader.java:435)
  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
  at java.lang.Thread.run(Thread.java:1012)
Caused by: java.lang.IllegalArgumentException
  at androidx.media3.common.util.Assertions.checkArgument(Assertions.java:40)
  at androidx.media3.common.util.ParsableByteArray.setPosition(ParsableByteArray.java:164)
  at androidx.media3.extractor.text.cea.Cea608Parser.parse(Cea608Parser.java:440)
```

PiperOrigin-RevId: 592876546
PiperOrigin-RevId: 593061829
These were missing in androidx@5211ff0

Without these, the `": null"` is still logged (but the `"Unexpected
IllegalStateException"` bit is not), because the **whole** string
concatenation is compared to null as the boolean condition of the
ternary, and this condition is always false (the result of a string
concatentation is never `null`) i.e. (with excess parentheses for
clarity):

```
(("Unexpected " + cause.getClass().getSimpleName() + cause.getMessage()) != null)
    ? ": " + cause.getMessage()
    : ""
```

Also add a test because obviously this isn't as simple as I'd thought.

PiperOrigin-RevId: 593079975
fix for Issue: androidx#829

Manual Testing: Viewed the transformer output file of previously problematic case in Exoplayer, Chrome, VLC, Quicktime and Safari and all showed the issue not to occur anymore. The newly produced output file can be found at https://github.com/androidx/media/assets/42352357/fdf105c1-9550-422f-b088-7900f655ac78

PiperOrigin-RevId: 593104752
This more closely matches the intended, documented behaviour of this
method. The previous implementation was incorrectly checking
`sampleDataEnd + newSampleSize`, but it's more correct to compare
`existingSampleDataLength + newSampleSize`, in order to handle the
case of non-zero `sampleDataStart`. We've already checked above whether
`newSampleSize` fits after `sampleDataEnd` without growing or
reshuffling the array, so no need to basically repeat that check.

In the case of handling one sample at a time, the previous
implementation resulted in `sampleData` growing by `sampleSize`
**every time** the pointers reached the end. With the new check for
`sampleDataStart == sampleDataEnd`, this is avoided by always writing
each new sample at the start of the array (because the previous sample
has already been consumed), meaning `sampleData` remains equal to
`sampleSize` without growing.

PiperOrigin-RevId: 593119927
PiperOrigin-RevId: 593143940
PiperOrigin-RevId: 593164068
Mp4Muxer does not support out of order B-frames. Currently it
silently writes out of order B-frames, producing an invalid file (with
negative sample durations).

Although `Mp4Extractor` is somehow able to process this invalid file and
`Exoplayer` is able to play it but that is unexpected.

The `sample.mp4` test file contains B frames. Other test files does not
contain `H264 video + AAC audio` format hence created a new test file by
running `sample.mp4` via `Transformer` after applying some effects.

PiperOrigin-RevId: 594016144
PiperOrigin-RevId: 594274620
Changes includes;
1. Public API to enable fMP4 and to pass fragment duration.
2. Added `FragmentedMp4Writer`.
3. Added logic to create fragments based on given fragment duration.
4. Write "moov" box only once in the beginning.
3. Add all the required boxes for current implementation.
4. Unit tests for all the new boxes.
5. E2E test for generating fMP4.

Note: The output file is un seek-able with this first implementation.
PiperOrigin-RevId: 594426486
When the 'when' timer of the notification is disabled
`DefaultMediaNotificationProvider` may set `C.TIME_UNSET`
as the time. Users reported problems on some devices with
this and the docs ask for an event time that probably
shouldn't be a negative number.

This change sets `0L` instead of `C.TIME_UNSET` when the
timer is disabled.

Issue: androidx#903

#minor-release

PiperOrigin-RevId: 594451074
The `Cea608DecoderTest` added here fails if re-ordering is removed from
`CeaDecoder`.

The `Cea608ParserTest` is added with `@Ignore` because there's currently
no re-ordering support in this part of the subtitle handling pipeline
(partly because there's no concept of 'current playback time', meaning
it's hard to know **when** to re-order).

PiperOrigin-RevId: 595320205
The `PlaybackService` of the demo app is declared
as `androidx.media3.session.MediaSessionService` instead
of `androidx.media3.session.MediaLibraryService`. While
this technically works, its confusing to do that in the
demo app.

Generally, apps that declare the legacy
`android.media.browse.MediaBrowserService` should also
declare `androidx.media3.session.MediaLibraryService`
and the demo app should reflect this common case.

Issue: androidx#672
PiperOrigin-RevId: 595320994
Matches the approach for video frame rate.

PiperOrigin-RevId: 595385930
icbaker and others added 22 commits April 2, 2024 16:41
The behaviour and docs of `playUntilPosition` were changed in
androidx@00c7a9b.

This change also affects `playUntilStartOfMediaItem` (since it delegates
to `playUntilPosition`), so the same doc change should also be made
here.

#minor-release

PiperOrigin-RevId: 607364897
(cherry picked from commit fa3212e)
Fixes an issue caused by no support for negative audio PTS and edit lists
in FrameworkMuxer, Android versions before 11

PiperOrigin-RevId: 607690507
(cherry picked from commit e43f966)
Issue: androidx#1087
PiperOrigin-RevId: 608534659
(cherry picked from commit 52c1d60)
The `PreloadMediaPeriod.selectTracksForPreloading` can be called for multiple times at the preloading stage (before the period is being played). For example, when the period resumes preloading. This change fix the assertion failure in `ProgressiveMediaPeriod.selectTracks` caused by the wrong implementation of `PreloadMediaPeriod.selectTracksForPreloading` when it is trying to retain the previously preloaded streams.

Also the `TrackSelectorResult` parameter is changed to a list of `ExoTrackSelection`. We should compare the selections only rather than considering the `RendererConfiguration` in the `TrackSelectorResult` to decide whether to retain the streams, as for preloading case the renderers haven't consumed the samples yet.

PiperOrigin-RevId: 609126868
(cherry picked from commit d952a06)
If render error occurs due to AudioTrack initialization failure in offload mode, then ExoPlayer should allow retry as subsequent attempt will be with DefaultAudioSink disabling offload.

PiperOrigin-RevId: 609304897
(cherry picked from commit 9046f2e)
If the reading period has already advanced and a track reselection procs that only affects the reading period media, then ExoPlayer may try and apply the reading period's track selection incorrectly unto the playing period. ExoPlayer should apply the playing period's track selection to the playing period instead.

PiperOrigin-RevId: 609375077
(cherry picked from commit 4192924)
We forgot to add it when we added AudioSink.release(). The commit
includes a test that ensures ForwardingAudioSink overrides all the
methods defined in the AudioSink interface.

PiperOrigin-RevId: 609402258
(cherry picked from commit 440d2ab)
This change makes ExoPlayerImplInternal.releaseInternal() unblock the
app thread if a runtime exception is thrown while releasing components
from the playback thread.

Before this change, if a runtime exception occurred during releasing
components in the playback thread, ExoPlayer.release() would wait for
`releaseTimeoutMs` and then raise a player error. With this change,
the player error is reported only when the playback thread is blocked
but if there is a runtime exception, the application thread is
unblocked.

The impact of this change is potentially fewer ANRs on
ExoPlayer.release() at the expense of less error reporting.

PiperOrigin-RevId: 609702549
(cherry picked from commit 0480eff)
Some devices supporting Performance Points for decoder coverage are missing coverage over the CDD requirements for H264. For these cases ExoPlayer should fall back to legacy resolution and frame rate support checks. If there is a stream evaluated as a PerformancePointCoverageResult of COVERAGE_RESULT_NO, then ExoPlayer checks for coverage of the 720p H264 CDD requirement.

Issue: google/ExoPlayer#10898

Issue: androidx#693

Issue: androidx#966
PiperOrigin-RevId: 609740128
(cherry picked from commit 23a301f)
This change aims to prioritise tracks that have a 'smooth enough for
video' frame rate, without always selecting the track with the highest
frame rate.

In particular MP4 files extracted from motion photos sometimes have two
HEVC tracks, with the higher-res one having a very low frame rate (not
intended for use in video playback). Before this change
`DefaultTrackSelector` would pick the low-fps, high-res track.

This change adds a somewhat arbitrary 10fps threshold for "smooth video
playback", meaning any tracks above this threshold are selected in
preference to tracks below it. Within the tracks above the threshold
other attributes are used to select the preferred track. We deliberately
don't pick the highest-fps track (over pixel count and bitrate), because
most users would prefer to see a 30fps 4k track over a 60fps 720p track.

This change also includes a test MP4 file, extracted from the existing
`jpeg/pixel-motion-photo-2-hevc-tracks.jpg` file by logging
`mp4StartPosition` in
[`MotionPhotoDescription.getMotionPhotoMetadata`](https://github.com/androidx/media/blob/b930b40a16c06318e43c81771fa2b1024bdb3f29/libraries/extractor/src/main/java/androidx/media3/extractor/jpeg/MotionPhotoDescription.java#L123)
and then using `dd`:

```
mp4StartPosition=2603594

$ dd if=jpeg/pixel-motion-photo-2-hevc-tracks.jpg \
    of=mp4/pixel-motion-photo-2-hevc-tracks.mp4 \
    bs=1 \
    skip=2603594
```

----

This solution is in addition to the `JpegMotionPhotoExtractor` change
made specifically for these two-track motion photos in
androidx@5266c71.
We will keep both changes, even though that change is not strictly
needed after this one, because adding the role flags helps to
communicate more clearly the intended usage of these tracks. This
change to consider FPS seems like a generally useful improvement to
`DefaultTrackSelector`, since it seems unlikely we would prefer a 5fps
video track over a 30fps one.

Issue: androidx#1051
PiperOrigin-RevId: 611015459
(cherry picked from commit c7e00b1)
When the controller replaces the current item, the masking position will be changed to the default position of the new item for a short while, before the correct position comes from the session. This will interrupt the current position fetched from the controller when the playback doesn't interrupted by the item replacing.

Issue: androidx#951

PiperOrigin-RevId: 611417539
(cherry picked from commit 1bdc58d)
PiperOrigin-RevId: 612485043
(cherry picked from commit bbdaf2b)
The removed check searched for a player command inside a list of
session commands, which is not allowed by the IntDef definition
and only worked because both types map to a Java int.

PiperOrigin-RevId: 612758442
(cherry picked from commit c79ac5b)
PiperOrigin-RevId: 612808322
(cherry picked from commit 3a43bd7)
PiperOrigin-RevId: 613156951
(cherry picked from commit a90a704)
#minor-release

PiperOrigin-RevId: 621828038
(cherry picked from commit d57229a)
Renderers may be enabled for subsequent media items as soon as the current media item's renderer's isEnded() returns true. When a renderer is being enabled and the player is 'playing', that renderer is also started. When playing a mixed playlist of images and content with audio & video, the player may skip some image items because the early-starting of the audio renderer causes a clock update.

A solution is to only start the "early-enabled" renderers at the point of media transition and add a condition on DefaultMediaClock to use the standalone clock when reading-ahead and the renderer clock source is not in a started state.

Issue: androidx#1017
PiperOrigin-RevId: 613231227
(cherry picked from commit 638b2a3)
Issue: androidx#1051
PiperOrigin-RevId: 613516802
(cherry picked from commit afacf2c)
PiperOrigin-RevId: 622189733
(cherry picked from commit 5fc35a6)
PiperOrigin-RevId: 622211426
(cherry picked from commit 2a5b9af)
@jdtremblay jdtremblay marked this pull request as ready for review April 22, 2024 16:48
Copy link
Member

@simonlary simonlary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui j'ai définitivement tout regarder chaque fichier. 😆

@jdtremblay
Copy link
Member Author

jdtremblay commented Apr 23, 2024

Je ferme la PR, nos changements sont trop difficile à suivre comme ça. On va plutôt faire une nouvelle branche mirego-main-1.3.1 à partir de la branche release et appliquer nos commits par dessus.

@jdtremblay jdtremblay closed this Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.