Skip to content

Commit

Permalink
Cherry-pick c8652ac. rdar://122842659
Browse files Browse the repository at this point in the history
    REGRESSION (271599@main): espn.com: Closed captions aren't displaying in videos
    https://bugs.webkit.org/show_bug.cgi?id=269847
    rdar://122842659

    Reviewed by Eric Carlson.

    In this video, we had 2 text tracks, all with a track id of 0.
    The reasons for this is that it contained a InbandMetadataTextTrack and a InbandTextTrack
    and the MediaPlayerPrivateAVFoundation weren't assigning them unique trackID which
    would confuse the search in the hash table trying to match a TrackID to a track.

    A TrackID for an inband text track is an abstract concept, it doesn't reflect anything
    relevant to the user (Except that it must be unique).
    We only need that TrackID to be unique between all the tracks in a given MediaPlayer.

    So we assign them a unique TrackID at creation.

    Add a HLS file that contains both a metadata and a subtitle track and ensure
    that cues are added to the proper one.

    * LayoutTests/http/tests/media/hls/track-webvtt-multitracks-expected.txt: Added.
    * LayoutTests/http/tests/media/hls/track-webvtt-multitracks.html: Added.
    * LayoutTests/http/tests/media/resources/hls/test-webvtt-multitracks.m3u8: Added.
    * Source/WebCore/platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.cpp:
    (WebCore::InbandMetadataTextTrackPrivateAVF::create):
    (WebCore::InbandMetadataTextTrackPrivateAVF::InbandMetadataTextTrackPrivateAVF):
    * Source/WebCore/platform/graphics/avfoundation/InbandMetadataTextTrackPrivateAVF.h:
    * Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.cpp:
    (WebCore::InbandTextTrackPrivateAVF::InbandTextTrackPrivateAVF):
    * Source/WebCore/platform/graphics/avfoundation/InbandTextTrackPrivateAVF.h:
    * Source/WebCore/platform/graphics/avfoundation/objc/InbandTextTrackPrivateAVFObjC.h:
    (WebCore::InbandTextTrackPrivateAVFObjC::create):
    * Source/WebCore/platform/graphics/avfoundation/objc/InbandTextTrackPrivateAVFObjC.mm:
    (WebCore::InbandTextTrackPrivateAVFObjC::InbandTextTrackPrivateAVFObjC):
    (WebCore::InbandTextTrackPrivateAVFObjC::id const): Deleted.
    * Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:
    (WebCore::MediaPlayerPrivateAVFoundationObjC::processMediaSelectionOptions):
    (WebCore::MediaPlayerPrivateAVFoundationObjC::processMetadataTrack):
    * Source/WebCore/platform/graphics/avfoundation/objc/OutOfBandTextTrackPrivateAVF.h:
    (WebCore::OutOfBandTextTrackPrivateAVF::create):
    (WebCore::OutOfBandTextTrackPrivateAVF::OutOfBandTextTrackPrivateAVF):

    Canonical link: https://commits.webkit.org/275201@main

Identifier: 272448.624@safari-7618-branch
  • Loading branch information
jyavenard authored and MyahCobbs committed Feb 24, 2024
1 parent 9049ab8 commit 4f03858
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

EXPECTED (video.textTracks.length == '2') OK
RUN(video.textTracks[1].mode = "showing")
EVENT(cuechange)
EXPECTED (video.textTracks[1].cues.length >= '1') OK
END OF TEST

26 changes: 26 additions & 0 deletions LayoutTests/http/tests/media/hls/track-webvtt-multitracks.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<!DOCTYPE html>
<html>
<head>
<script src=../../media-resources/video-test.js></script>
<script>
window.addEventListener('load', async event => {
video = document.getElementById('video');
waitForAndFail(video, 'error');
video.src = "../resources/hls/test-webvtt-multitracks.m3u8";
await Promise.all([
video.play(),
waitFor(video.textTracks, 'addtrack', true),
testExpectedEventually('video.textTracks.length', 2, '==', 5000),
]);
run('video.textTracks[1].mode = "showing"');
await waitFor(video.textTracks[1], 'cuechange');
testExpected("video.textTracks[1].cues.length", 1, '>=');

endTest();
});
</script>
</head>
<body>
<video id="video" muted loop></video>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#EXTM3U

#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="subtitles",LANGUAGE="en-US",NAME="English",AUTOSELECT=YES,DEFAULT=YES,URI="subtitles/prog_style.m3u8"
#EXT-X-STREAM-INF:BANDWIDTH=634451,CODECS="mp4a.40.2, avc1.4d401e",RESOLUTION=640x480,SUBTITLES="subtitles"
metadata/prog_index.m3u8
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@

namespace WebCore {

Ref<InbandMetadataTextTrackPrivateAVF> InbandMetadataTextTrackPrivateAVF::create(InbandTextTrackPrivate::Kind kind, InbandTextTrackPrivate::CueFormat cueFormat, TrackID id)
Ref<InbandMetadataTextTrackPrivateAVF> InbandMetadataTextTrackPrivateAVF::create(InbandTextTrackPrivate::Kind kind, TrackID id, InbandTextTrackPrivate::CueFormat cueFormat)
{
return adoptRef(*new InbandMetadataTextTrackPrivateAVF(kind, cueFormat, id));
return adoptRef(*new InbandMetadataTextTrackPrivateAVF(kind, id, cueFormat));
}

InbandMetadataTextTrackPrivateAVF::InbandMetadataTextTrackPrivateAVF(InbandTextTrackPrivate::Kind kind, InbandTextTrackPrivate::CueFormat cueFormat, TrackID id)
InbandMetadataTextTrackPrivateAVF::InbandMetadataTextTrackPrivateAVF(InbandTextTrackPrivate::Kind kind, TrackID id, InbandTextTrackPrivate::CueFormat cueFormat)
: InbandTextTrackPrivate(cueFormat)
, m_kind(kind)
, m_id(id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ struct IncompleteMetaDataCue {

class InbandMetadataTextTrackPrivateAVF : public InbandTextTrackPrivate {
public:
static Ref<InbandMetadataTextTrackPrivateAVF> create(Kind, CueFormat, TrackID = 0);
static Ref<InbandMetadataTextTrackPrivateAVF> create(Kind, TrackID, CueFormat);

~InbandMetadataTextTrackPrivateAVF();

Expand All @@ -58,7 +58,7 @@ class InbandMetadataTextTrackPrivateAVF : public InbandTextTrackPrivate {
void flushPartialCues();

private:
InbandMetadataTextTrackPrivateAVF(Kind, CueFormat, TrackID);
InbandMetadataTextTrackPrivateAVF(Kind, TrackID, CueFormat);

#if !RELEASE_LOG_DISABLED
const char* logClassName() const final { return "InbandMetadataTextTrackPrivateAVF"; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,15 @@ namespace WebCore {

AVFInbandTrackParent::~AVFInbandTrackParent() = default;

InbandTextTrackPrivateAVF::InbandTextTrackPrivateAVF(AVFInbandTrackParent* owner, CueFormat format)
InbandTextTrackPrivateAVF::InbandTextTrackPrivateAVF(AVFInbandTrackParent* owner, TrackID trackID, CueFormat format)
: InbandTextTrackPrivate(format)
, m_owner(owner)
, m_pendingCueStatus(None)
, m_index(0)
, m_hasBeenReported(false)
, m_seeking(false)
, m_haveReportedVTTHeader(false)
, m_trackID(trackID)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class InbandTextTrackPrivateAVF : public InbandTextTrackPrivate {
public:
virtual ~InbandTextTrackPrivateAVF();

TrackID id() const final { return m_trackID; }

void setMode(InbandTextTrackPrivate::Mode) override;

int trackIndex() const override { return m_index; }
Expand Down Expand Up @@ -79,7 +81,7 @@ class InbandTextTrackPrivateAVF : public InbandTextTrackPrivate {
virtual bool readNativeSampleBuffer(CFArrayRef nativeSamples, CFIndex, RefPtr<JSC::ArrayBuffer>&, MediaTime&, CMFormatDescriptionRef&);

protected:
InbandTextTrackPrivateAVF(AVFInbandTrackParent*, CueFormat);
InbandTextTrackPrivateAVF(AVFInbandTrackParent*, TrackID, CueFormat);

Ref<InbandGenericCue> processCueAttributes(CFAttributedStringRef);
void processAttributedStrings(CFArrayRef, const MediaTime&);
Expand Down Expand Up @@ -110,6 +112,7 @@ class InbandTextTrackPrivateAVF : public InbandTextTrackPrivate {
bool m_hasBeenReported;
bool m_seeking;
bool m_haveReportedVTTHeader;
const TrackID m_trackID { 0 };
};

} // namespace WebCore
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,13 @@ namespace WebCore {

class InbandTextTrackPrivateAVFObjC : public InbandTextTrackPrivateAVF {
public:
static Ref<InbandTextTrackPrivateAVFObjC> create(AVFInbandTrackParent* player, AVMediaSelectionGroup *group, AVMediaSelectionOption *selection, InbandTextTrackPrivate::CueFormat format)
static Ref<InbandTextTrackPrivateAVFObjC> create(AVFInbandTrackParent* player, AVMediaSelectionGroup *group, AVMediaSelectionOption *selection, TrackID trackID, InbandTextTrackPrivate::CueFormat format)
{
return adoptRef(*new InbandTextTrackPrivateAVFObjC(player, group, selection, format));
return adoptRef(*new InbandTextTrackPrivateAVFObjC(player, group, selection, trackID, format));
}

~InbandTextTrackPrivateAVFObjC() = default;

TrackID id() const override;

InbandTextTrackPrivate::Kind kind() const override;
bool isClosedCaptions() const override;
bool isSDH() const override;
Expand All @@ -64,7 +62,7 @@ class InbandTextTrackPrivateAVFObjC : public InbandTextTrackPrivateAVF {
AVMediaSelectionOption *mediaSelectionOption() const { return m_mediaSelectionOption.get(); }

protected:
InbandTextTrackPrivateAVFObjC(AVFInbandTrackParent*, AVMediaSelectionGroup *, AVMediaSelectionOption *, InbandTextTrackPrivate::CueFormat);
InbandTextTrackPrivateAVFObjC(AVFInbandTrackParent*, AVMediaSelectionGroup *, AVMediaSelectionOption *, TrackID, InbandTextTrackPrivate::CueFormat);

RetainPtr<AVMediaSelectionGroup> m_mediaSelectionGroup;
RetainPtr<AVMediaSelectionOption> m_mediaSelectionOption;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ - (id)optionID;

namespace WebCore {

InbandTextTrackPrivateAVFObjC::InbandTextTrackPrivateAVFObjC(AVFInbandTrackParent* player, AVMediaSelectionGroup *group, AVMediaSelectionOption *selection, InbandTextTrackPrivate::CueFormat format)
: InbandTextTrackPrivateAVF(player, format)
InbandTextTrackPrivateAVFObjC::InbandTextTrackPrivateAVFObjC(AVFInbandTrackParent* player, AVMediaSelectionGroup *group, AVMediaSelectionOption *selection, TrackID trackID, InbandTextTrackPrivate::CueFormat format)
: InbandTextTrackPrivateAVF(player, trackID, format)
, m_mediaSelectionGroup(group)
, m_mediaSelectionOption(selection)
{
Expand All @@ -63,14 +63,6 @@ - (id)optionID;
InbandTextTrackPrivateAVF::disconnect();
}

TrackID InbandTextTrackPrivateAVFObjC::id() const
{
if (m_mediaSelectionOption)
return [[m_mediaSelectionOption optionID] unsignedLongLongValue];
ASSERT_NOT_REACHED();
return 0;
}

InbandTextTrackPrivate::Kind InbandTextTrackPrivateAVFObjC::kind() const
{
if (!m_mediaSelectionOption)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3126,13 +3126,13 @@ void determineChangedTracksFromNewTracksAndOldItems(MediaSelectionGroupAVFObjC*

#if ENABLE(AVF_CAPTIONS)
if ([option outOfBandSource]) {
m_textTracks.append(OutOfBandTextTrackPrivateAVF::create(this, option));
m_textTracks.append(OutOfBandTextTrackPrivateAVF::create(this, option, m_currentTextTrackID++));
m_textTracks.last()->setHasBeenReported(true); // Ignore out-of-band tracks that we passed to AVFoundation so we do not double-count them
continue;
}
#endif

m_textTracks.append(InbandTextTrackPrivateAVFObjC::create(this, legibleGroup, option, InbandTextTrackPrivate::CueFormat::Generic));
m_textTracks.append(InbandTextTrackPrivateAVFObjC::create(this, legibleGroup, option, m_currentTextTrackID++, InbandTextTrackPrivate::CueFormat::Generic));
}

processNewAndRemovedTextTracks(removedTextTracks);
Expand All @@ -3143,7 +3143,7 @@ void determineChangedTracksFromNewTracksAndOldItems(MediaSelectionGroupAVFObjC*
if (m_metadataTrack)
return;

m_metadataTrack = InbandMetadataTextTrackPrivateAVF::create(InbandTextTrackPrivate::Kind::Metadata, InbandTextTrackPrivate::CueFormat::Data);
m_metadataTrack = InbandMetadataTextTrackPrivateAVF::create(InbandTextTrackPrivate::Kind::Metadata, m_currentTextTrackID++, InbandTextTrackPrivate::CueFormat::Data);
m_metadataTrack->setInBandMetadataTrackDispatchType("com.apple.streaming"_s);
if (auto player = this->player())
player->addTextTrack(*m_metadataTrack);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ namespace WebCore {

class OutOfBandTextTrackPrivateAVF : public InbandTextTrackPrivateAVF {
public:
static Ref<OutOfBandTextTrackPrivateAVF> create(AVFInbandTrackParent* player, AVMediaSelectionOption* selection)
static Ref<OutOfBandTextTrackPrivateAVF> create(AVFInbandTrackParent* player, AVMediaSelectionOption* selection, TrackID trackID)
{
return adoptRef(*new OutOfBandTextTrackPrivateAVF(player, selection));
return adoptRef(*new OutOfBandTextTrackPrivateAVF(player, selection, trackID));
}

void processCue(CFArrayRef, CFArrayRef, const MediaTime&) override { }
Expand All @@ -49,8 +49,8 @@ class OutOfBandTextTrackPrivateAVF : public InbandTextTrackPrivateAVF {
AVMediaSelectionOption* mediaSelectionOption() const { return m_mediaSelectionOption.get(); }

protected:
OutOfBandTextTrackPrivateAVF(AVFInbandTrackParent* player, AVMediaSelectionOption* selection)
: InbandTextTrackPrivateAVF(player, InbandTextTrackPrivate::CueFormat::Generic)
OutOfBandTextTrackPrivateAVF(AVFInbandTrackParent* player, AVMediaSelectionOption* selection, TrackID trackID)
: InbandTextTrackPrivateAVF(player, trackID, InbandTextTrackPrivate::CueFormat::Generic)
, m_mediaSelectionOption(selection)
{
}
Expand Down

0 comments on commit 4f03858

Please sign in to comment.