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

Inconsistent currentPosition via MediaController if skip silence is enabled #1035

Closed
Digipom opened this issue Jan 26, 2024 · 9 comments
Closed
Assignees

Comments

@Digipom
Copy link

Digipom commented Jan 26, 2024

Version

Media3 1.2.1

Devices that reproduce the issue

Pixel 7

Reproducible in the demo app?

Not tested

Reproduction steps

Can be reproduced by setting skip silence on.

Expected result

Position should immediately jump according to skipped parts due to skip silence.

Actual result

There's sometimes a delay of up to a few seconds, as also observed in #765. This seems to still be affecting MediaController. I'm also not seeing onPositionDiscontinuity being emitted even when the time does jump, as mentioned in the commit.

Media

Any speech file that has pauses.

@Digipom
Copy link
Author

Digipom commented Jan 26, 2024

I tested this on 1.3.0-alpha01 (which contains the fix for #765) and the issue is improved but not entirely fixed. Some skips don't have the discontinuity emitted.

I sent a test media file to android-media-github@google.com.

For example, between "Fun in Osaka" and "The Osaka demonstration was great fun.", we get a discontinuity, but between that one and the next segment, we don't. The shift in playback position to the next segment is delayed.

@tianyif
Copy link
Contributor

tianyif commented Jan 30, 2024

Hi @Digipom,

Thanks for reporting! You're right, the last fix is an improvement, but unfortunately cannot report every time when there is a period of silence skipped. The complexities reside in the two facts:

a. The silence period can be thin;
b. The noisy period can also be thin;

Both of them will result the event onPositionDiscontinuity to be emitted too frequently. Based on the tradeoff, the standards that we are using to report the silence skipping discontinuity are:

  • When a silence period is detected, we wait for 100ms to check if there is another silence period coming. If yes, this means that the two silence periods are close enough, so we can accumulate them into one. And if not, this means that any later silence period is far from the current accumulated period, we can consider to report them. This is to address the above complexity b).
  • For the accumulated period that we consider to report, there is another check on its length. We only report the accumulated silence period that is no less than 1s. This is to address the above complexity a).

So there can be a corner case that, the accumulated silence period is nearly but still less than 1s, then it gets dropped from reporting onPositionDiscontinuity.

Hope this helps!

@Digipom
Copy link
Author

Digipom commented Jan 31, 2024

@tianyif without being familiar with the internals, is there a way to get a deterministic jump or for the skip silence detector to notify you exactly when the skips happen, so we can get deterministic discontinuities? The reason why I'm asking is that I'm building a transcription app and it's important that the current media position actually reflect the real position within the file, so we can highlight the correct text segments. Even if the behavior is better, it still leads to some lag.

If it's not possible currently, I'd like to keep the issue open as a feature request in that case. Thank you very much.

@tianyif
Copy link
Contributor

tianyif commented Feb 1, 2024

@Digipom Thanks for the information!

The current length threshold for reporting an accumulated silence period is 1s, with which we ignore any accumulated silence period shorter than it. It could create a significant shift between the controller current position and the real position of the player indeed. So would if reducing it, say 300ms, can help? I experimented on the demo-session app with your content, and think the sent onPositionDiscontinuity can reflect speaker's “change breath” at the full stops, and some of the commas. Could you please let me know this solution sound acceptable for your case?

@Digipom
Copy link
Author

Digipom commented Feb 1, 2024

@tianyif I would say, if it's possible to be deterministic, updating for every skip would be ideal. If we ignore skips under 300ms, then if we have small skips repeated over and over, isn't it still possible to end up with a multiple-second discrepancy between the current media time and the actual media time? When would it get synchronized up again?

Otherwise, certainly 300ms will probably be an improvement over 1000ms.

Would it perhaps be possible to expose the configuration so that the application developers could further customize this behavior?

Thank you.

copybara-service bot pushed a commit that referenced this issue Feb 8, 2024
Issue: #1035
#minor-release
PiperOrigin-RevId: 605361126
copybara-service bot pushed a commit to google/ExoPlayer that referenced this issue Feb 8, 2024
Issue: androidx/media#1035
#minor-release
PiperOrigin-RevId: 605361126
SheenaChhabra pushed a commit that referenced this issue Feb 9, 2024
Issue: #1035
#minor-release
PiperOrigin-RevId: 605361126
(cherry picked from commit 9b0cdde)
@tianyif
Copy link
Contributor

tianyif commented Mar 4, 2024

Hi @Digipom,

We pushed a commit earlier to improve the skip silence reporting logic, which ensures that the discrepancy at the controller from the real playback position won't exceed 300ms. Could you please try and see if this solution fits your case? I'm closing this issue for now, but feel free to re-open it if there are more questions.

@tianyif tianyif closed this as completed Mar 4, 2024
@Digipom
Copy link
Author

Digipom commented Mar 4, 2024

@tianyif Sure I can test it. Which version should I use to test?

@tianyif
Copy link
Contributor

tianyif commented Mar 4, 2024

@Digipom It went with 1.3.0.

@Digipom
Copy link
Author

Digipom commented Mar 4, 2024

@tianyif I tested and the behavior is much improved. Thank you!

@androidx androidx locked and limited conversation to collaborators May 4, 2024
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