WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
213888
[MSE][GStreamer] Don't skip samples past media duration in AppendPipeline
https://bugs.webkit.org/show_bug.cgi?id=213888
Summary
[MSE][GStreamer] Don't skip samples past media duration in AppendPipeline
Alicia Boya García
Reported
2020-07-02 09:19:37 PDT
appsinkNewSample() contained code to skip samples whose presentation time starts after media duration. This is paradoxical, because later sourceBufferPrivateDidReceiveSample() extends media duration whenever the presentation end time of the sample is past the original one. It does not sound reasonable that samples extending the duration are okay on one case but outright rejected in the other. Also, if it was about skipping samples, sourceBufferPrivateDidReceiveSample() could do it itself, and already does in other cases. For all these reasons I was very doubtful of the need for this if() and indeed removing this condition didn't cause any new test failure.
Attachments
Patch
(2.80 KB, patch)
2020-07-02 09:20 PDT
,
Alicia Boya García
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Alicia Boya García
Comment 1
2020-07-02 09:20:30 PDT
Created
attachment 403372
[details]
Patch
Enrique Ocaña
Comment 2
2020-07-02 09:32:34 PDT
After discussing this removal with Alicia, checking where the original code came from before being upstreamed[1] (I can't recall any rationale for discarding samples beyond the duration), and Alicia checking that it doesn't break any YouTube TV test[2] nor LayoutTest, I agree with removing the code block. [1]
https://github.com/eocanha/webkit/commit/86764b20df56#diff-1a0340714c802e905fa36d804906e36eR3270
[2]
https://ytlr-cert.appspot.com/2019/main.html
EWS
Comment 3
2020-07-06 00:42:35 PDT
Committed
r263962
: <
https://trac.webkit.org/changeset/263962
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 403372
[details]
.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug