[Webkit-unassigned] [Bug 179690] [MSE] Use correct range end checks in sourceBufferPrivateDidReceiveSample()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=179690

--- Comment #6 from Jer Noble <jer.noble at apple.com> ---
(In reply to Alicia Boya García from comment #5)
> (In reply to Jer Noble from comment #2)
> > Thanks for writing these tests. But I'm curious what the behavior of this
> > change is when faced with appends like:
> > 
> > // PTS, duration
> > [0, 2]
> > [2, 2]
> > [1, 2]
> > 
> > I would argue that the correct results would be:
> > 
> > buffered.length == 1
> > buffered.start(0) == 1
> > buffered.end(0) == 3
> > 
> > I'm not sure what this patch's results would be, as it stands.
> 
> It would produce the results the spec says, whether good or bad...
> 
> In this case it would depend on whether the DTSs are consecutive so the
> frames are considered part of the same Coded Frame Group.
> 
> If they are all part of the same Coded Frame Group, none of the conditions
> would evaluate true so no frames would be deleted:
> 
> 1.13. If last decode timestamp for track buffer is unset (false) ...
> 1.14.a. If highest end timestamp for track buffer is not set (false).
> 1.14.b. If highest end timestamp for track buffer is set (true) and less
> than or equal to presentation timestamp (false).
> 
> {PTS({0/1000 = 0.000000}), DTS({0/1000 = 0.000000}), duration({2000/1000 =
> 2.000000}), flags(1), generation(0)}
> {PTS({2000/1000 = 2.000000}), DTS({1000/1000 = 1.000000}),
> duration({2000/1000 = 2.000000}), flags(1), generation(0)}
> {PTS({1000/1000 = 1.000000}), DTS({2000/1000 = 2.000000}),
> duration({2000/1000 = 2.000000}), flags(1), generation(0)}
> 
> If at least the last frame starts a new Coded Frame Group (e.g. by having a
> lesser DTS than the previous one):
> 
> 1.13. (enters in the condition but the frame is not removed because its PTS
> is not within the 1 microsecond window [PTS, PTS+1µs) ).
> 1.14.a. If highest end timestamp for track buffer is not set: (true)
>   Remove all coded frames from track buffer that have a presentation
> timestamp greater than or equal to presentation timestamp and less than
> frame end timestamp. (removes the previous frame)
> 1.14.b. If highest end timestamp for track buffer is set (false) ...
> 
> {PTS({0/1000 = 0.000000}), DTS({0/1000 = 0.000000}), duration({2000/1000 =
> 2.000000}), flags(1), generation(0)}
> {PTS({1000/1000 = 1.000000}), DTS({0/1000 = 0.000000}), duration({2000/1000
> = 2.000000}), flags(1), generation(0)}

Yuck. I really don't like the idea of having overlapping samples in this case. I would propose that we willfully violate the spec in this case in one of two ways:

Option 1: Remove the overlapping sample.  This obeys (I believe) the spirit of the specification and ensures that no samples can overlap inside the track buffer.

Option 2: Truncate the duration of the overlapping sample.  This solves the overlapping sample problem without actually removing the sample itself.  Has the possible benefit of matching GStreamer's behavior, where duration is ignored anyway.

Option 1 would be easier to implement; it's a cross platform change to the sample append code.

Option 2 would require changes to MediaSample and all its derived classes to add a new virtual "setDuration()" method.

WDYT?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171115/42691405/attachment.html>