Bug 135164

Summary: [MSE][Mac] In SourceBufferPrivateAVFObjC::abort(), support reseting parser to the last appended initialization segment.
Product: WebKit Reporter: Jer Noble <jer.noble>
Component: New BugsAssignee: Jer Noble <jer.noble>
Status: NEW ---    
Severity: Normal CC: commit-queue, eric.carlson, joeyparrish, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 135163    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
eric.carlson: review+
Patch for landing
none
Patch for landing none

Description Jer Noble 2014-07-22 10:57:26 PDT
[MSE][Mac] In SourceBufferPrivateAVFObjC::abort(), support reseting parser to the last appended initialization segment.
Comment 1 Jer Noble 2016-09-27 16:46:02 PDT
Created attachment 290022 [details]
Patch
Comment 2 Jer Noble 2016-09-27 16:49:23 PDT
rdar://problem/28438637
Comment 3 Eric Carlson 2016-09-27 16:57:59 PDT
Comment on attachment 290022 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290022&action=review

Looks like you forgot a few files :-)

> Source/WebCore/ChangeLog:17
> +        and will block until the the previous append() operation completes.

Nit: the the

> Source/WebCore/ChangeLog:20
> +        SourceBufferPrivateAVFObjC to be reset after an abort(), so make that ivar a @property. Rather than passing a

Nit: "a @property" -> "an @property"
Comment 4 Jer Noble 2016-09-27 17:01:13 PDT
Created attachment 290026 [details]
Patch
Comment 5 Eric Carlson 2016-09-27 17:10:15 PDT
Comment on attachment 290026 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=290026&action=review

> Source/WebCore/ChangeLog:17
> +        and will block until the the previous append() operation completes.

Nit: "the the"

> Source/WebCore/ChangeLog:20
> +        SourceBufferPrivateAVFObjC to be reset after an abort(), so make that ivar a @property. Rather than passing a

Nit: "a @property" -> "an @property"

> Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:675
> +    dispatch_group_wait(m_isAppendingGroup.get(), DISPATCH_TIME_FOREVER);

Is it really safe to wait forever?
Comment 6 Jer Noble 2016-09-27 17:48:13 PDT
(In reply to comment #5)
> Comment on attachment 290026 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=290026&action=review
> 
> > Source/WebCore/ChangeLog:17
> > +        and will block until the the previous append() operation completes.
> 
> Nit: "the the"

Ok.

> > Source/WebCore/ChangeLog:20
> > +        SourceBufferPrivateAVFObjC to be reset after an abort(), so make that ivar a @property. Rather than passing a
> 
> Nit: "a @property" -> "an @property"

Ok.

> > Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:675
> > +    dispatch_group_wait(m_isAppendingGroup.get(), DISPATCH_TIME_FOREVER);
> 
> Is it really safe to wait forever?

Now that I think of it, no, it's not.  The parsing thread may be blocked on the main thread providing it a AVStreamSession for EME-backed media elements. Apart from that, however, it should be fine.
Comment 7 Jer Noble 2016-09-27 17:51:10 PDT
Created attachment 290034 [details]
Patch for landing
Comment 8 Jer Noble 2016-09-27 18:01:29 PDT
Created attachment 290040 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2016-09-28 07:55:10 PDT
Comment on attachment 290040 [details]
Patch for landing

Clearing flags on attachment: 290040

Committed r206518: <http://trac.webkit.org/changeset/206518>