Bug 230380

Summary: [Cocoa] Support in-band chapter tracks
Product: WebKit Reporter: Eric Carlson <eric.carlson>
Component: MediaAssignee: Eric Carlson <eric.carlson>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, ews-watchlist, glenn, jer.noble, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch for landing none

Description Eric Carlson 2021-09-16 16:37:49 PDT
Support in-band chapter tracks
Comment 1 Radar WebKit Bug Importer 2021-09-16 16:37:59 PDT
<rdar://problem/83218578>
Comment 2 Eric Carlson 2021-09-27 11:12:54 PDT
Created attachment 439370 [details]
Patch
Comment 3 Eric Carlson 2021-09-27 15:11:16 PDT
Created attachment 439398 [details]
Patch
Comment 4 Eric Carlson 2021-09-27 17:07:00 PDT
Created attachment 439419 [details]
Patch
Comment 5 Jer Noble 2021-09-28 08:54:05 PDT
Comment on attachment 439419 [details]
Patch

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

r=me with nits and a question.

> Source/WebCore/platform/graphics/avfoundation/objc/InbandChapterTrackPrivateAVFObjC.h:39
> +class InbandChapterTrackPrivateAVFObjC : public InbandTextTrackPrivate {

Nit: final.

> Source/WebCore/platform/graphics/avfoundation/objc/InbandChapterTrackPrivateAVFObjC.h:51
> +    int trackIndex() const override { return m_index; }

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/InbandChapterTrackPrivateAVFObjC.h:59
> +    AtomString inBandMetadataTrackDispatchType() const override { return "com.apple.chapters"_s; }

Ditto.

> Source/WebCore/platform/graphics/avfoundation/objc/InbandChapterTrackPrivateAVFObjC.mm:74
> +                [item loadValuesAsynchronouslyForKeys:@[@"value"] completionHandler:[this, protectedThis = Ref { *this }, item = retainPtr(item), createChapterCue = WTFMove(createChapterCue), chapterNumber, identifier] () mutable {

WTFMove(createChapterCue) seems really weird here; won't that mean only the first iteration through the for loop has a valid lambda?
Comment 6 Eric Carlson 2021-09-28 13:11:23 PDT
Created attachment 439509 [details]
Patch for landing
Comment 7 Eric Carlson 2021-09-29 08:11:34 PDT
Comment on attachment 439509 [details]
Patch for landing

The iOS API test failure is unrelated.
Comment 8 EWS 2021-09-29 08:32:07 PDT
Committed r283217 (242259@main): <https://commits.webkit.org/242259@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439509 [details].