-
Notifications
You must be signed in to change notification settings - Fork 154
[GST][MSE] Make sure to extend only the very first sample when DTS = … #1601
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
base: wpe-2.46
Are you sure you want to change the base?
[GST][MSE] Make sure to extend only the very first sample when DTS = … #1601
Conversation
…0 and PTS > 0. In the absence of an edit list, ensure that WebKit’s AppendPipeline extends only a single sample. This fixes the handling of invalid streams with multiple DTS = 0 samples. Modifying PTS values for all of them results in unexpected behavior, playback issues, PTS reordering, etc. The problem was spotted in a video web application that uses TS -> MSE transmuxing.
cd573ff to
e360c3b
Compare
|
Alternatively, since Edit lists are supported, we can consider removing that code block or enable it for GST < 1.16 only This was spot in AFV app with dailymotion/hls.js transmuxer. There were two samples with DTS=0 and PTS > 0 |
…0 and PTS > 0 https://bugs.webkit.org/show_bug.cgi?id=305858 Reviewed by NOBODY (OOPS!). In the absence of an edit list, ensure that WebKit’s AppendPipeline extends only a single sample. This fixes the handling of invalid streams with multiple DTS = 0 samples. Modifying PTS values for all of them results in unexpected behavior, playback issues, PTS reordering, etc. The problem was spotted in a video web application that uses TS -> MSE transmuxing. See: WebPlatformForEmbedded/WPEWebKit#1601 Original author: Andrzej Surdej <101130014+asurdej-comcast@users.noreply.github.com> * Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.cpp: (WebCore::AppendPipeline::appsinkNewSample): Only extend first sample if no other sample has ever been extended before, or since the last parser reset. Set the hasExtendedFirstSample flag for the track on first extension. (WebCore::AppendPipeline::resetParserState): Reset the hasExtendedFirstSample for all tracks. * Source/WebCore/platform/graphics/gstreamer/mse/AppendPipeline.h: (WebCore::AppendPipeline::Track::Track): Add hasExtendedFirstSample flag.
|
I think it's better to try to introduce this patch, since it may also benefit upstream, where edit list support is reverted (but should be eventually reintroduced). The patch looks good, although having two (or more) samples with DTS=0 can cause other problems (regardless of this patch): Since both DTS are equal, we don't have much guarantees about the stability of the DecodeOrderSampleMap in SourceBufferPrivate holding the samples. Any of the two samples with DTS=0 could be returned earlier than the other one depending on storage ordering or just luck. Patch submitted upstream for review as https://bugs.webkit.org/show_bug.cgi?id=305858 / WebKit/WebKit#56917. |
…0 and PTS > 0.
In the absence of an edit list, ensure that WebKit’s AppendPipeline extends only a single sample.
This fixes the handling of invalid streams with multiple DTS = 0 samples. Modifying PTS values for all of them results in unexpected behavior, playback issues, PTS reordering, etc.
The problem was spotted in a video web application that uses TS -> MSE transmuxing.