Bug 213888

Summary: [MSE][GStreamer] Don't skip samples past media duration in AppendPipeline
Product: WebKit Reporter: Alicia Boya García <aboya>
Component: WebKitGTKAssignee: Alicia Boya García <aboya>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, calvaris, cgarcia, eocanha, ews-watchlist, gustavo, menard, pnormand, vjaquez
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Alicia Boya García 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.
Comment 1 Alicia Boya García 2020-07-02 09:20:30 PDT 
Created attachment 403372 [details]
Patch
Comment 2 Enrique Ocaña 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
Comment 3 EWS 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].