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

Provide ForwardingMediaSource to simplify custom implementations #7279

Closed
stari4ek opened this issue Apr 22, 2020 · 11 comments
Closed

Provide ForwardingMediaSource to simplify custom implementations #7279

stari4ek opened this issue Apr 22, 2020 · 11 comments

Comments

@stari4ek
Copy link

[REQUIRED] Use case description

Current implementation of HlsMediaSource starts with default position:

We attempt to set the default start position to be at least twice the target duration behind the live edge.

So, if HLS has lots of segments or they are long enough - default start position will be pretty far from it's 0. This is desired behavior when playing "live" media.
But, for example, "catchup" of IPTV services is implemented in the way, when url identifies specific time (wall clock), and when player starts playback - it should start from 0 instead of "live edge" to match target wall clock.

Alternatives considered

I have tried:

Proposed solution

I have tried:

I think that some functionality can be provided by library itself:

  • fully functional CustomStartPositionSource
  • or at least easy-to-extend delegating media source. Otherwise CompositeMediaSource should be used (the same as it's used by ClippingMediaSource), which looks a bit of overkill.
@stari4ek
Copy link
Author

Proof of concept implementation looks like:

public class CustomStartPositionSource extends CompositeMediaSource<Void> {

    private final MediaSource mWrappedSource;
    private final long mStartPos;

    private Timeline mTimeline;

    public CustomStartPositionSource(MediaSource wrappedSource, long startPositionMs) {
        this.mWrappedSource = wrappedSource;
        this.mStartPos = C.msToUs(startPositionMs);
    }

    // MediaSource

    @Nullable
    @Override
    public Object getTag() {
        return mWrappedSource.getTag();
    }

    // CompositeMediaSource

    @Override
    protected void prepareSourceInternal(@Nullable TransferListener mediaTransferListener) {
        super.prepareSourceInternal(mediaTransferListener);
        prepareChildSource(/* id= */ null, mWrappedSource);
    }

    @NonNull
    @Override
    public MediaPeriod createPeriod(
            @NonNull MediaPeriodId id,
            @NonNull Allocator allocator,
            long startPositionUs) {

        return mWrappedSource.createPeriod(id, allocator, startPositionUs);
    }

    @Override
    public void releasePeriod(@NonNull MediaPeriod mediaPeriod) {
        mWrappedSource.releasePeriod(mediaPeriod);
    }

    @Override
    protected void releaseSourceInternal() {
        super.releaseSourceInternal();
        mTimeline = null;
    }

    @Override
    protected void onChildSourceInfoRefreshed(
            @NonNull Void id,
            @NonNull MediaSource mediaSource,
            @NonNull Timeline timeline) {

        mTimeline = new CustomStartPositionTimeline(timeline, mStartPos);
        refreshSourceInfo(mTimeline);
    }

    private static final class CustomStartPositionTimeline extends ForwardingTimeline {

        private final long defaultPositionUs;

        CustomStartPositionTimeline(Timeline wrapped, long defaultPositionUs) {
            super(wrapped);
            this.defaultPositionUs = defaultPositionUs;
        }

        @NonNull
        @Override
        public Window getWindow(
                int windowIndex,
                @NonNull Window window,
                long defaultPositionProjectionUs) {

            super.getWindow(windowIndex, window, defaultPositionProjectionUs);
            window.defaultPositionUs = defaultPositionUs;
            return window;
        }
    }
}

@stari4ek
Copy link
Author

Another one, purely on BaseMediaSource

public class CustomStartPositionSource extends BaseMediaSource {

    private final MediaSource mWrappedSource;
    private final long mStartPosUs;

    private Timeline mTimeline;

    private final MediaSource.MediaSourceCaller mCaller;

    CustomStartPositionSource(@NonNull MediaSource wrappedSource, long startPositionMs) {
        this.mWrappedSource = wrappedSource;
        this.mStartPosUs = C.msToUs(startPositionMs);

        mCaller = (source, timeline) -> {
            mTimeline = new CustomStartPositionTimeline(timeline, mStartPosUs);
            refreshSourceInfo(mTimeline);
        };
    }

    // BaseMediaSource

    @Override
    protected void prepareSourceInternal(@Nullable TransferListener mediaTransferListener) {

        mWrappedSource.prepareSource(mCaller, mediaTransferListener);
        if (!isEnabled()) {
            mWrappedSource.disable(mCaller);
        }
    }

    @Override
    protected void enableInternal() {
        mWrappedSource.enable(mCaller);
    }

    @Override
    protected void disableInternal() {
        mWrappedSource.disable(mCaller);
    }

    @Override
    protected void releaseSourceInternal() {
        mWrappedSource.releaseSource(mCaller);
        mTimeline = null;
    }

    // MediaSource

    @Nullable
    public Timeline getInitialTimeline() {
        return mWrappedSource.getInitialTimeline();
    }

    @Override
    public boolean isSingleWindow() {
        return mWrappedSource.isSingleWindow();
    }

    @Nullable
    @Override
    public Object getTag() {
        return mWrappedSource.getTag();
    }

    @Override
    public void maybeThrowSourceInfoRefreshError() throws IOException {
        mWrappedSource.maybeThrowSourceInfoRefreshError();
    }

    @NonNull
    @Override
    public MediaPeriod createPeriod(
            @NonNull MediaPeriodId id,
            @NonNull Allocator allocator,
            long startPositionUs) {

        return mWrappedSource.createPeriod(id, allocator, startPositionUs);
    }

    @Override
    public void releasePeriod(@NonNull MediaPeriod mediaPeriod) {
        mWrappedSource.releasePeriod(mediaPeriod);
    }

    private static final class CustomStartPositionTimeline extends ForwardingTimeline {

        private final long defaultPositionUs;

        CustomStartPositionTimeline(@NonNull Timeline wrapped, long defaultPositionUs) {
            super(wrapped);
            this.defaultPositionUs = defaultPositionUs;
        }

        @NonNull
        @Override
        public Window getWindow(
                int windowIndex,
                @NonNull Window window,
                long defaultPositionProjectionUs) {

            super.getWindow(windowIndex, window, defaultPositionProjectionUs);
            window.defaultPositionUs = defaultPositionUs;
            return window;
        }
    }
}

@kim-vde
Copy link
Contributor

kim-vde commented Apr 23, 2020

Can you clarify why you would extend CompositeMediaSource or BaseMediaSource instead of MediaSource (as in #2403)?

@kim-vde kim-vde self-assigned this Apr 23, 2020
@stari4ek
Copy link
Author

Honestly speaking diving into whole sources/timelines/periods/windows is pretty hard stuff, when you have no experience with it. And since source code in the example is a bit out of date - I was looking for existing solutions, which wraps another MediaSource. So there were SingleSampleMediaSource, ClippingMediaSource, test implementation in delegatingMediaSourceApproach I started with.
Maybe, after doing CompositeMediaSource -> BaseMediaSource, it will be much easier to get it right purely on MediaSource for me.
But, looking to ClippingMediaSource I was thinking why it's based on CompositeMediaSource and if there is any real need in it, cause it looks like wrapping source, but there is no ready-to-use base class for it.

@stari4ek
Copy link
Author

ok, here is MediaSource based:

public class CustomStartPositionSource implements MediaSource {

    private final MediaSource mWrappedSource;
    private final long mStartPosUs;

    public CustomStartPositionSource(@NonNull MediaSource wrappedSource, long startPositionMs) {
        this.mWrappedSource = wrappedSource;
        this.mStartPosUs = C.msToUs(startPositionMs);
    }

    // MediaSource

    @Override
    public void addEventListener(@NonNull Handler handler,
                                 @NonNull MediaSourceEventListener eventListener) {

        mWrappedSource.addEventListener(handler, eventListener);
    }

    @Override
    public void removeEventListener(@NonNull MediaSourceEventListener eventListener) {
        mWrappedSource.removeEventListener(eventListener);
    }

    @Override
    public void addDrmEventListener(@NonNull Handler handler,
                                    @NonNull DrmSessionEventListener eventListener) {
        mWrappedSource.addDrmEventListener(handler, eventListener);
    }

    @Override
    public void removeDrmEventListener(@NonNull DrmSessionEventListener eventListener) {
        mWrappedSource.removeDrmEventListener(eventListener);
    }

    @Override
    @Nullable
    public Timeline getInitialTimeline() {
        return mWrappedSource.getInitialTimeline();
    }

    @Override
    public boolean isSingleWindow() {
        return mWrappedSource.isSingleWindow();
    }

    @Nullable
    @Override
    public Object getTag() {
        return mWrappedSource.getTag();
    }

    @Override
    public void prepareSource(
            @NonNull MediaSourceCaller caller,
            @Nullable TransferListener mediaTransferListener) {

        mWrappedSource.prepareSource(
            (source, timeline) ->
                caller.onSourceInfoRefreshed(source, new CustomStartPositionTimeline(timeline, mStartPosUs)),
            mediaTransferListener
        );
    }

    @Override
    public void maybeThrowSourceInfoRefreshError() throws IOException {
        mWrappedSource.maybeThrowSourceInfoRefreshError();
    }

    @Override
    public void enable(@NonNull MediaSourceCaller caller) {
        mWrappedSource.enable(caller);
    }

    @NonNull
    @Override
    public MediaPeriod createPeriod(
            @NonNull MediaPeriodId id,
            @NonNull Allocator allocator,
            long startPositionUs) {

        return mWrappedSource.createPeriod(id, allocator, startPositionUs);
    }

    @Override
    public void releasePeriod(@NonNull MediaPeriod mediaPeriod) {
        mWrappedSource.releasePeriod(mediaPeriod);
    }

    @Override
    public void disable(@NonNull MediaSourceCaller caller) {
        mWrappedSource.disable(caller);
    }

    @Override
    public void releaseSource(@NonNull MediaSourceCaller caller) {
        mWrappedSource.releaseSource(caller);
    }

    private static final class CustomStartPositionTimeline extends ForwardingTimeline {

        private final long defaultPositionUs;

        CustomStartPositionTimeline(@NonNull Timeline wrapped, long defaultPositionUs) {
            super(wrapped);
            this.defaultPositionUs = defaultPositionUs;
        }

        @NonNull
        @Override
        public Window getWindow(
                int windowIndex,
                @NonNull Window window,
                long defaultPositionProjectionUs) {

            super.getWindow(windowIndex, window, defaultPositionProjectionUs);
            window.defaultPositionUs = defaultPositionUs;
            return window;
        }
    }
}

So it solves my problem, but I think that custom "default playback pos" strategy for HLS can be pretty useful.
And having some delegating media source base class would remove boilerplate and maybe simplify some existing implementations (ClippingMediaSource?)

@kim-vde
Copy link
Contributor

kim-vde commented Apr 23, 2020

  • About adding a CustomStartPositionMediaSource: I will mark this issue as duplicate of Allow playlist items to specify a start position #6373 (as a single media item is a special case of a playlist).
  • About adding a ForwardingMediaSource in order to simplify custom media sources: @tonihei, can you give your opinion on that? Do you think this would be helpful for many use cases?

@tonihei
Copy link
Collaborator

tonihei commented Apr 23, 2020

Maybe, after doing CompositeMediaSource -> BaseMediaSource, it will be much easier to get it right purely on MediaSource for me.
But, looking to ClippingMediaSource I was thinking why it's based on CompositeMediaSource and if there is any real need in it, cause it looks like wrapping source, but there is no ready-to-use base class for it.

When wrapping one or more other media sources you should use CompositeMediaSource (as in your first code example) because it takes care of some event forwarding logic and does not reinvent the wheel.

About adding a ForwardingMediaSource in order to simplify custom media sources: @tonihei, can you give your opinion on that? Do you think this would be helpful for many use cases?

Sounds like a helpful addition for cases where MediaSources forward to exactly one other source and only make small adjustments. I've seen some examples that repeat the same boilerplate forwarding code and would benefit from this. So I'll mark as an enhancement to implement this.

@tonihei tonihei changed the title Ability to start HLS playback from custom position Provide ForwardingMediaSource to simplify custom implementations Apr 23, 2020
@stari4ek
Copy link
Author

because it takes care of some event forwarding logic and does not reinvent the wheel.

@tonihei, there is message forwarding in CompositeMediaSource, but all of messages come from MediaSourceEventListener, DrmSessionEventListener. So, I'd expect that all of it is covered with direct forwarding of add|removeEventListener, add|removeDrmEventListener to wrapped source.
Please correct me if I'm wrong here.

@tonihei
Copy link
Collaborator

tonihei commented Apr 23, 2020

Should probably work. It's still easier to rely on an existing wrapper for boilerplate code mostly because code changes in the future will automatically update the base classes, whereas if you implement the interface directly, you need to update your implementation manually.

@stari4ek
Copy link
Author

stari4ek commented May 2, 2020

@tonihei. You were right. :)
Even when it looks simple - it is not. Got memory leak due not-matching callers (HLS media source wrapped by CustomStartPositionSource based on MediaSource from my example above)

@tonihei
Copy link
Collaborator

tonihei commented Jan 12, 2023

This issue has been addressed by androidx/media#123.

@tonihei tonihei closed this as completed Jan 12, 2023
@google google locked and limited conversation to collaborators Mar 14, 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

4 participants