You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Let AudioSink indicate if playback speed changes are supported to avoid unnecessary live speed adjustments #10865
Apply attached patch "patch-r2.15.0.txt" to branch at tag r2.15.0. This will generate logs referenced in "Actual result" section below, or you can use your own logging.
Play a working (AAC audio) stream and compare with Non-Working (AC3 stream).
Expected result
Playback speed for AC3 streams should adjust to push liveOffsetUs up to idealTargetLiveOffsetUs.
Actual result
Note in the logs:
For AAC: liveOffsetUs → currentTargetLiveOffsetUs and adjustedPlaybackSpeed → 1.0 eventually
For AC3: liveOffsetUs does not move towards currentTargetLiveOffsetUs and adjustedPlaybackSpeed stays at 1.3
In both cases above currentTargetLiveOffsetUs →idealTargetLiveOffsetUs, which should be expected behavior. So AAC will eventually end up at the playback position of idealTargetLiveOffsetUs, but not AC3.
The same behavior is seen in r.2.18.X as well as androidx/media (some adjustments are required to trigger/force Playback Speed Control because of commit, #9329)
Media
patch-r2.15.0.txt
Media streaming information will be sent separately via email.
Bug Report
You will email the zip file produced by adb bugreport to dev.exoplayer@gmail.com after filing this issue.
liveOffsetUs does not move towards currentTargetLiveOffsetUs and adjustedPlaybackSpeed stays at 1.3
This sounds odd and logically impossible :) Could you also share the logs you obtained with the attached patch? I'm curious to see how the values change (or not change) and I don't have an NVidea Shield available right now.
I have used a slightly different patch than previously uploaded, which I think would be more useful. I have included it in Issue-10865-Dec-14-22-log.zip along with logs for an AAC run as well as an AC3 run (the files are named appropriately).
The logs are filtered, please let me know if you'd like completely unfiltered logs.
From the logs at least one conclusion can be made that mediaClock.setPlaybackParameters(playbackInfo.playbackParameters.withSpeed(adjustedSpeed)) is likely not working as expected for AC3, or is being overwritten.
BTW @tonihei I noticed the Live Offset feature is turned off by default with a comment it is not so useful for non-LL streaming. There is one place it is very useful, the hospitality use case (many TV's in a bar, health club, etc, all watching the same event)
Issue-10865-Dec-16-22-log.zip
I have another data point that I feel could be useful. Current logs were generated by the changes identical to the ones in the previous comment PLUS a single line change to DefaultAudioSink.java. It is useful to compare these logs with the ones posted in the previous comment by me.
With this change you will notice:
mediaClock.setPlaybackParameters(speed) is being preserved between calls
liveOffsetUs starts to track towards idealTargetLiveOffsetUs.
Even though the liveOffsetUs starts to track there are other side effects like terrible lip sync that shows up after a few minutes. The change to DefaultAudioSink.java is just to help understand the behavior of this issue - its not a proposal of a real change :-) . I'm curious to hear back what you think!
Thanks for the all the logs and description. It looks like DefaultAudioSink just doesn't support changing the playback speed for this combination of device and format (maybe it's using tunneling or some other passthrough mode?). This is basically what you've overwritten in your second patch.
Then I think this is really just working as intended because the speed can't be adjusted in the output path. We don't know that in advance at the moment because the clock (=DefaultAudioSink by default) can be injected by apps and there is a delay before we even find out. Is this behavior causing any problems during playback besides not having live offset adjustment? I think at the time of writing the code we considered this issue and said that it will needlessly try to set the adjusted speed, but since it's a no-op, it also doesn't really matter.
We could improve the code and resolve the confusion you had by adding a method to MediaClock that lets callers check if playback speed adjustments are supported in the current configuration. This then would allow the player to NOT call the live playback speed adjustment if the speed can't be adjusted anyway.
(maybe it's using tunneling or some other passthrough mode?)
I am using stock ExoPlayer@r2.15.0 - the only changes I've made is what I also attached in the patch. So, tunneling is off. The only dominant condition check in DefaultAudioSink.shouldApplyAudioProcessorPlaybackParameters() is MimeTypes.AUDIO_RAW.equals(configuration.inputFormat.sampleMimeType) which will always evaluate to false for AC3 and the other checks don't matter.
So, what I understand is that, as long as the input stream is AC3, we will not be able to adjust playback position by speed adjusting, and that is expected behavior. In that case, and other cases too, it may make sense to add an API that lets callers check if playback speed adjustments are supported in the current configuration. Please let me know if I should keep this issue around for the API addition or can be closed.
changed the title [-]Playback speed does not adjust when audio is AC3[/-][+]Let AudioSink indicate if playback speed changes are supported to avoid unnecessary live speed adjustments[/+]on Jan 5, 2023
Activity
tonihei commentedon Dec 14, 2022
This sounds odd and logically impossible :) Could you also share the logs you obtained with the attached patch? I'm curious to see how the values change (or not change) and I don't have an NVidea Shield available right now.
psharma676 commentedon Dec 14, 2022
Issue-10865-Dec-14-22-log.zip
stevemayhew commentedon Dec 14, 2022
BTW @tonihei I noticed the Live Offset feature is turned off by default with a comment it is not so useful for non-LL streaming. There is one place it is very useful, the hospitality use case (many TV's in a bar, health club, etc, all watching the same event)
psharma676 commentedon Dec 16, 2022
Issue-10865-Dec-16-22-log.zip
I have another data point that I feel could be useful. Current logs were generated by the changes identical to the ones in the previous comment PLUS a single line change to DefaultAudioSink.java. It is useful to compare these logs with the ones posted in the previous comment by me.
With this change you will notice:
Even though the liveOffsetUs starts to track there are other side effects like terrible lip sync that shows up after a few minutes. The change to DefaultAudioSink.java is just to help understand the behavior of this issue - its not a proposal of a real change :-) . I'm curious to hear back what you think!
tonihei commentedon Dec 21, 2022
Thanks for the all the logs and description. It looks like
DefaultAudioSinkjust doesn't support changing the playback speed for this combination of device and format (maybe it's using tunneling or some other passthrough mode?). This is basically what you've overwritten in your second patch.Then I think this is really just working as intended because the speed can't be adjusted in the output path. We don't know that in advance at the moment because the clock (=
DefaultAudioSinkby default) can be injected by apps and there is a delay before we even find out. Is this behavior causing any problems during playback besides not having live offset adjustment? I think at the time of writing the code we considered this issue and said that it will needlessly try to set the adjusted speed, but since it's a no-op, it also doesn't really matter.We could improve the code and resolve the confusion you had by adding a method to
MediaClockthat lets callers check if playback speed adjustments are supported in the current configuration. This then would allow the player to NOT call the live playback speed adjustment if the speed can't be adjusted anyway.setPlaybackParameters() not overwriten by live adjusment
psharma676 commentedon Dec 23, 2022
Should have a relation to #10882
I am using stock ExoPlayer@r2.15.0 - the only changes I've made is what I also attached in the patch. So, tunneling is off. The only dominant condition check in DefaultAudioSink.shouldApplyAudioProcessorPlaybackParameters() is MimeTypes.AUDIO_RAW.equals(configuration.inputFormat.sampleMimeType) which will always evaluate to false for AC3 and the other checks don't matter.
So, what I understand is that, as long as the input stream is AC3, we will not be able to adjust playback position by speed adjusting, and that is expected behavior. In that case, and other cases too, it may make sense to add an API that lets callers check if playback speed adjustments are supported in the current configuration. Please let me know if I should keep this issue around for the API addition or can be closed.
tonihei commentedon Jan 5, 2023
I'll mark it as an enhancement to avoid the unnecessary constant updates that don't work anyway.
[-]Playback speed does not adjust when audio is AC3[/-][+]Let AudioSink indicate if playback speed changes are supported to avoid unnecessary live speed adjustments[/+]setPlaybackParameters() not overwriten by live adjusment
setPlaybackParameters() not overwriten by live adjusment
setPlaybackParameters() not overwriten by live adjusment