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

Allow ClippingMediaSource in a ConcatenatingMediaSource2 by adjusting start timestamps #11226

Closed
thomas-coldwell opened this issue Jun 24, 2023 · 10 comments
Assignees

Comments

@thomas-coldwell
Copy link

Hi I am trying to achieve the following media composition with two videos - a first video with no clipping applied that is then concatenated with a second clip that has the first 1000ms of playback clipped. The mode for this looks something like:

val firstVideo = createMediaSourceFromUrl(firstVideoSource)
val secondVideo = createMediaSourceFromUrl(secondVideoSource, clipStartMs: 1000)

val finalMediaSource = ConcatenatingMediaSource2.Builder()
  .add(firstVideo)
  .add(secondVideo)
  .build()

However, I get the following error "Can't concatenate windows. A window has a non-zero offset in a period." from this line in the ConcatenatingMediaSource2 implementation. I also noticed at the top of the class it specifically says it excludes ClippingMediaSource's from being concat.

I'm trying to understand the way this class works a bit better and why the non-zero offset of the clipped media source prevents it from being concatenated and was wondering if anyone might be able to share some insights on this or if there is a way to achieve this (or the steps I might take to make my own Concat class that would support this)?

Thank you in advance! 🙌

@thomas-coldwell
Copy link
Author

From what I have looked at so far this seems to be more of an issue with how the ClippingMediaSource is constructed than with the logic of the ConcatenatingMediaSource2. The clipping media source class creates a clipped sample stream which on its own should be fine, but then the non-zero offset record ing the ClippingMediaPeriod that holds this stream appears to cause the issue.

With that in mind could I create a new clipping media source class (or add a flag to the current one) that ensures the sample stream is clipped, but then has a media period that treats the media as if it is just the clipped subsequence e.g. it has an offset of 0, but the sample stream is in fact reading from the start of whichever subsequence we specify?

@tonihei tonihei self-assigned this Jun 26, 2023
@tonihei
Copy link
Collaborator

tonihei commented Jun 26, 2023

Sounds like you explained everything yourself already :)

The media model of ExoPlayer uses "windows" for the user-facing entities and "periods" for the more technical individual stream instances. A ConcatenatingMediaSource2 creates a single "window" with many "periods". The main issue is that this media model only allows a non-zero start position for the first "period" in a "window". This is mainly for historical reasons where the terminology and the model constraints came from DASH periods.

The ideal way to fix this is to make the media model more flexible and allow non-zero offsets for all periods. However, this is tricky to achieve as lots of code in ExoPlayer and many existing apps has been built under the current assumptions.

We considered adding a workaround for the specific problem you mention (ClippingMediaSource inside a ConcatenatingMediaSource2) because it's quite common. The workaround would essentially do what you describe (move the timestamps of the ClippingMediaSource to start from zero). Unfortunately this still leaves some cases unsolved, because once you do this shifting of timestamps, you can no longer easily update the source to start from another timestamp. For example, change the clipping start time, or use the setup to play a live stream with a moving window that changes its start time every few seconds. Still worth to add this workaround to cover the most common cases at least. I'll mark the issue as an enhancement to track this work!

@tonihei tonihei changed the title Why can I not add a ClippingMediaSource to a ConcatenatingMediaSource2? Allow ClippingMediaSource in a ConcatenatingMediaSource2 by adjusting start timestamps Jun 26, 2023
@thomas-coldwell
Copy link
Author

Awesome! Thanks for the detailed explanation @tonihei 🙌

For now I'll try setup a custom ClippingMediaSource class that does this as I don't really need to update the clip start position or use any live streams. But will be great to have this in core for some common use-cases like this! Thanks again!

@thomas-coldwell
Copy link
Author

Just checking I've got this correct @tonihei - by adjusting the timestamps that would mean setting up a custom media source, media period and finally a sample stream in which I'd create an offset for the time the samples should be read at? I've tried setting this up but haven't had much success with it yet and hoping you might be able to easily point me in the right general direction 🙏

Thanks in advance!

@tonihei
Copy link
Collaborator

tonihei commented Jun 29, 2023

It may be complicated to get this right. There is already a TimeOffsetMediaPeriod you could copy (but please remove this max call, which is actually a bug we are currently fixing). And then you also need a WrappingMediaSource that creates these periods. I'm afraid I can't provide more detailed one to one support for this type of customization. Alternatively, you can also wait until we implement it in the library.

@thomas-coldwell
Copy link
Author

Thanks @tonihei - appreciate your time and the pointers here! 🙌

@pawelnuzka
Copy link

@thomas-coldwell Did you manage to accomplish this? If so, could you share some sample code?
@tonihei Is there any timeline for the official fix?

@tonihei
Copy link
Collaborator

tonihei commented Aug 25, 2023

Had a look into this issue this week. We'll reuse the TimeOffsetMediaPeriod automatically from within ConcatenatingMediaSource2 to solve this issue in the library. The commits for this probably appear here within the next week or so.

copybara-service bot pushed a commit to androidx/media that referenced this issue Sep 8, 2023
This makes it reusable for other MediaSource/Periods in the same
package.

Issue: google/ExoPlayer#11226
PiperOrigin-RevId: 563687935
copybara-service bot pushed a commit that referenced this issue Sep 8, 2023
This makes it reusable for other MediaSource/Periods in the same
package.

Issue: #11226
PiperOrigin-RevId: 563687935
copybara-service bot pushed a commit to androidx/media that referenced this issue Sep 8, 2023
The class currently disallows offsets of periods in their windows
except for the very first window. This is not necessary because
we can use TimeOffsetMediaPeriod to eliminate the offset if needed.
This makes the class more useful for many use cases, in particular
for using it with ClippingMediaSource.

Issue: google/ExoPlayer#11226
PiperOrigin-RevId: 563702120
copybara-service bot pushed a commit that referenced this issue Sep 8, 2023
The class currently disallows offsets of periods in their windows
except for the very first window. This is not necessary because
we can use TimeOffsetMediaPeriod to eliminate the offset if needed.
This makes the class more useful for many use cases, in particular
for using it with ClippingMediaSource.

Issue: #11226
PiperOrigin-RevId: 563702120
@tonihei tonihei closed this as completed Sep 8, 2023
microkatz pushed a commit to v-novaltd/androidx-media that referenced this issue Sep 21, 2023
This makes it reusable for other MediaSource/Periods in the same
package.

Issue: google/ExoPlayer#11226
PiperOrigin-RevId: 563687935
microkatz pushed a commit to v-novaltd/androidx-media that referenced this issue Sep 21, 2023
The class currently disallows offsets of periods in their windows
except for the very first window. This is not necessary because
we can use TimeOffsetMediaPeriod to eliminate the offset if needed.
This makes the class more useful for many use cases, in particular
for using it with ClippingMediaSource.

Issue: google/ExoPlayer#11226
PiperOrigin-RevId: 563702120
@pawelnuzka
Copy link

@tonihei Do you have any info when this is released in an updated version of the library?

@tonihei
Copy link
Collaborator

tonihei commented Sep 25, 2023

This will be part of 1.2.0, probably beginning of November roughly. (But there will be alpha/beta/rc versions before that)

microkatz pushed a commit to hugohlln/media that referenced this issue Sep 29, 2023
This makes it reusable for other MediaSource/Periods in the same
package.

Issue: google/ExoPlayer#11226
PiperOrigin-RevId: 563687935
microkatz pushed a commit to hugohlln/media that referenced this issue Sep 29, 2023
The class currently disallows offsets of periods in their windows
except for the very first window. This is not necessary because
we can use TimeOffsetMediaPeriod to eliminate the offset if needed.
This makes the class more useful for many use cases, in particular
for using it with ClippingMediaSource.

Issue: google/ExoPlayer#11226
PiperOrigin-RevId: 563702120
@google google locked and limited conversation to collaborators Nov 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants