Incorporate MediaSourceEventListener.onLoadStarted with retryCount pa…
…rameter with backward compatibility
I think in its current form this stops calling any existing implementations of AnalyticsListener.onLoadStarted(EventTime, LoadEventInfo, MediaLoadData) (the one that's now deprecated). I checked this by adding some logging locally to this override in EventLogger and I see the logging before this change, but not after it.
We need to keep these implementations working - because although they're marked as deprecated, deprecated things are still expected to work.
Ensuring both the old and new callbacks continue working, without accidentally multiplying the calls, is somewhat tricky (you mentioned you experimented with putting some duplication in the EventDispatcher implementation, but this results in re-duplicating the events when they are forwarded inside e.g. CompositeMediaSource).
We can put the duplication in the callsites instead (i.e. in each MediaSource implementation). One question then becomes: how do we ensure that any custom MediaSource implementations call the new method (since we can't change them)? You could argue that this doesn't matter, because an app with a custom MediaSource impl also owns their custom AnalyticsListener impl, so they can migrate either both or neither - and therefore remain compatible with themselves.
I think in its current form this stops calling any existing implementations of
AnalyticsListener.onLoadStarted(EventTime, LoadEventInfo, MediaLoadData)(the one that's now deprecated). I checked this by adding some logging locally to this override inEventLoggerand I see the logging before this change, but not after it.We need to keep these implementations working - because although they're marked as deprecated, deprecated things are still expected to work.
Ensuring both the old and new callbacks continue working, without accidentally multiplying the calls, is somewhat tricky (you mentioned you experimented with putting some duplication in the
EventDispatcherimplementation, but this results in re-duplicating the events when they are forwarded inside e.g.CompositeMediaSource).We can put the duplication in the callsites instead (i.e. in each
MediaSourceimplementation). One question then becomes: how do we ensure that any customMediaSourceimplementations call the new method (since we can't change them)? You could argue that this doesn't matter, because an app with a customMediaSourceimpl also owns their customAnalyticsListenerimpl, so they can migrate either both or neither - and therefore remain compatible with themselves.
Thanks for the feedback and makes sense. I think duplicating the calls at each callsite makes sense to me since this is treating the updated API as a new API entirely. This seems cleaner too. I will adjust this