Fix to allow Audio timestampMaster causes stream startup delay #8952
Comments
This comment has been minimized.
This comment has been minimized.
|
Assuming the best fix is to return to using video must master timestamps. This is the best fix I believe, and the timestamp master certainly should not change during playback, only on track selection if the track that was timestamp master is unselected. This statement seems an interesting path:
This could work, at least for our use case, the trick is we need to refresh the timestamp master's playlist in order to have it pickup the correct updated DSN. Here's why If you mean to do this the HlsSampleStreamWrapper, that is @Override
public boolean continueLoading(long positionUs) {
if (loadingFinished || loader.isLoading() || loader.hasFatalError()) {
return false;
}
...
chunkSource.getNextChunk(
positionUs,
loadPositionUs,
chunkQueue,
/* allowEndOfStream= */ prepared || !chunkQueue.isEmpty(),
nextChunkHolder);
boolean endOfStream = nextChunkHolder.endOfStream;
Chunk loadable = nextChunkHolder.chunk;
// The loadable is null if the ChunkSource determined the DSN had changed and it was not a master
...
// So this code would refresh playlist, which keeps the continue loading happening
if (loadable == null) {
if (playlistUrlToLoad != null) {
callback.onPlaylistRefreshRequired(playlistUrlToLoad);
}
return false;
}The problem is refreshing the non-master playlist will not help. We need these two things to happen before continuing loading of this non-master rendition:
Its possible the logic in I'll experiment with this a bit and let you know. |
Nono, the proposal is that HlsSampleStreamWrapper.getNextLoadPosition asks its HlsChunksSource for the position of the next segment to load, and if that segment belongs to an uninitialized discontinuity sequence, and the chunk source is not an adjuster master, then HlsChunkSource will return an "infinite" value so that the composite sequenceable loader keeps loading from the master until the new discontinuity sequence number is initialized. Does this make sense? |
|
yes, that makes much more sense. As part of the Un-Reported discontinuity handling I was looking at making a hook for the I'm assuming you want to add a new sentinel value returned from public static final long LOAD_POSITION_BLOCKED = Long.MAX_VALUEIf this is returned, you need to either stay in the inner loop or break out and return true (so loading will callback): do {
madeProgressThisIteration = false;
long nextLoadPositionUs = getNextLoadPositionUs();
if (nextLoadPositionUs == C.TIME_END_OF_SOURCE) {
break;
}
for (SequenceableLoader loader : loaders) {
long loaderNextLoadPositionUs = loader.getNextLoadPositionUs();
boolean isLoaderBehind =
loaderNextLoadPositionUs != C.TIME_END_OF_SOURCE
&& loaderNextLoadPositionUs <= positionUs;
if (loaderNextLoadPositionUs == nextLoadPositionUs || isLoaderBehind) {
madeProgressThisIteration |= loader.continueLoading(positionUs);
}
}
madeProgress |= madeProgressThisIteration;
} while (madeProgressThisIteration);So, yes I think this will work great. Obviously the contract of the I'll implement it and test it with our use case, I'll push a branch as well if you still have the sources that reproduce Issue #8700 as well, or if you can share them with me I can re-test with this fix too. The more I think about it the timestamp master should never change after playback starts. Other nasty things can happen if the adjustment value changes from video to audio or visa-versa, basically there will be a gap where there is no A/V sync which will cause playback stalls of one stream or the other. The rest of the changes @ojw28 made to |
|
Certainly @ojw28, sorry for the confusion, the off topic comments are still linked above. And yes, from my read of #8850 the only similarity to this issue is the root cause is the same. My questions for this issue remains the same:
Thanks! |
|
We've submitted 4013612 to fix some other issues introduced by 6f8a8fb. I don't think it does anything to address the issue here though. What's happening in this case is that there are two segments (one audio, one video) that need to be loaded to start playback. The HLS playlists suggest that they both start at exactly the same point in time, but the audio segment appears to contain PTS values starting ~1 second earlier than the video segment. Which ever segment "wins" the race to initialize
As for how we should actually go about fixing this: I'm not sure that changing how the |
|
I'll have a look at 4013612 and add it to our testing.
This is exactly correct. In the old design, the video always set the time so audio before that time is discarded. The discard happens in the buffer.timeUs = timesUs[relativeReadIndex];
if (buffer.timeUs < startTimeUs) {
buffer.addFlag(C.BUFFER_FLAG_DECODE_ONLY);
}This happens on starting playback or a seek, easy of these of course reset the Interesting point on
Happy to do this. I'll send a URL to the dev email list. The biggest drawback to audio mastering is that video first frame render is un-conditional (complicated logic in I agree the Also, we need to consider what the tunneling codecs will do with this, as here we have no control over the render sequence. We have seen where BRCM codec will stutter out bursts of frames at random frame rates when presented with large time offsets like this. Lastly, issue #5024 looks exactly like this one, so feel free to combine or mark them as a duplicate. Thanks for opening the discussion on this, it does seem like some way to select how we clip the streams to bring them into alignment might be a better solution. Currently the |
|
I think you may be looking at the wrong demo URL, I sent a new pair to the developer mailing list. There are no PROGRAM-DATE-TIME tags, basically the sample is 60 seconds of VOD. Try with disabling chunkless prepare and turning off CC. Basically the selection of timestamp master is left to chance depending on the first playlist to start load of a media chunk. As you can see from the logs below, choosing audio as master (for this sample set at least) costs nearly double the time to first frame and start of playback.
Which ever load is not master will be forced to wait to even make the HTTP request for the segment, if this is video then obviously this will impact first frame time. One solution for this (if we decide to continue to randomize master) would be to block the first sample commit rather then the segment request (this way at least both reads are in progress) Here's the log data, added log where TS master is chosen in Audio MasterVideo Master |
The difference is that this issue is dealing with demuxed audio and video segments, where-as #5024 is asking about audio and video muxed into a single TS stream. It may be that we can fix both issues in the same way, which is why I was referring to it as being similar. I think an ideal fix would probably have this property. However, it's not necessarily the case that a fix for this issue will definitely fix #5024, or vice-versa. E.g., reverting how
I think there's agreement that aligning audio and video to start from the same place is preferable, both for this case and for the case in #5024. What we haven't figured out yet is what the best way of achieving that is, and whether it makes sense to fix them separately or together. |
|
That sounds good, thanks again for spending some time with this, lots of good ideas come from discussions like this. BTW, I think there is a simple fix for this:
I've got a build of code that move the call to As for the trimming / aligning functionality....
Yes, either:
Our BRCM guru (Sada, @sneelavara) tells me that the Broadcom codec has a similar function that discards audio timestamped before the first valid video frame. |
Sending a pull request for this sounds good to me. Note that you'll probably need to start passing a boolean down through |
|
@stevemayhew - Do you plan on sending a pull request for the first issue (as per my comment above)? Thanks! |
|
@ojw28 I've got it without the boolean now, I'm adding that and testing with it. I've been slowed down fixing some bugs with live playback that came from the |
|
Here's some background from Apple Developer Notes why the transcoders and packagers do this. Trimming should occur on SampleStream until first video frame is committed, that's the current thought. Like the |
If you're referring to the need for priming samples, then I don't think it does explain why audio segments would contain samples starting from ~1 second earlier than the advertised segment start time, as we were seeing according to #8952 (comment). As far as I'm aware, the priming duration will always be much smaller than that (somewhere in the order of ~20ms). |
The patch, allow audio master, commit 6f8a8f, fixes stall issues with ordering of updates of the DSN in segments across demuxed renditions however the cost is the seconds of delay on channel change or stream startup, a bad user experience.
To reproduce this bug start playback of any HLS live stream where audio timestamps precede video in the sequence of segments.
Expected:
Actual:
MediaCodecVideoRendererforces first frame inprocessOutputBuffer()This change to allow audio variants to reset timestamp adjustment fixes issues we have with live playback and marked discontinuities, so any fix must still address that issue. The bug here is simple.
TimestampAdjusteris picked up)TimestampAdjuster(cached version, or just the time ordering of the origin writing the playlists) so playback freezesBecause audio and video are kept in step by the
CompositeSequenceableLoader(not allowing timeline progress) then playback is frozen forever.Note the video rendition playlist will update to the new DSN, but the player does not see it.
The text was updated successfully, but these errors were encountered: