[DAI] Errors thrown by `SdkImpl.prototype.onAddTrack`

[alexypdu]

Description

The following error is thrown by SdkImpl.prototype.onAddTrack:

videojs-ima/src/dai/sdk-impl.js

Lines 134 to 146 in 187056e

	SdkImpl.prototype.onAddTrack = function(event) {
	const track = event.track;
	if (track.kind === 'metadata') {
	track.mode = 'hidden';
	track.addEventListener('cuechange', (e) => {
	for (const cue of track.activeCues_) {
	const metadata = {};
	metadata[cue.value.key] = cue.value.data;
	this.streamManager.onTimedMetadata(metadata);
	}
	});
	}
	};

And the root cause is a typo on this line:

videojs-ima/src/dai/sdk-impl.js

Line 139 in 187056e

for (const cue of track.activeCues_) {

where it should be track.activeCues instead.

[Kiro705]

Hi @alexypdu ,

Thanks for raising this issue. The fix has been made in commit 59c70b0.

The change will be included in the next plugin release.

Thank you,
Jackson
IMA SDK team

[Kiro705]

Hello @alexypdu ,

This issue should not be resolved in v2.5.0 of the plugin. I'm going to close this issue, but feel free to re-open it as needed.

Thank you,
Jackson
IMA SDK team

[alexypdu]

Hello @Kiro705 I do not have permissions to reopen this issue but I just realized that although native Web API TextTrack has property activeCues, Video.js emulated TextTrack has both properties activeCues and activeCues_ (and accessing textTrack.activeCues directly doesn't work), which I believed was why the videojs-ima plugin targeted activeCues_ in the first place.

By default, Video.js emulates text tracks on most browsers and defers that to native Web APIs on desktop Safari and iOS browsers (source).

videojs-ima should support both use cases but I'm not sure what's the best way to address the issue. A quick work around would be to check both track.activeCues and track.activeCues_ (or check the prototype chain).

[alexypdu]

Hello @Kiro705 I think I figured out the issue. Video.js's TextTrackCueList doesn't implement [Symbol.iterator] and hence track.activeCues is not iterable; on the other hand, track.activeCues_ as a private member is just a regular array. Good news is the class implements getters for length and [index] so we can do:

const GOOGLE_DAI_TIMED_METADATA_KEY = 'TXXX';

// use arrow function to keep the correct `this` context
track.addEventListener('cuechange', (e) => {
  const activeCues = track.activeCues;  
  
  for (let i = 0; i < activeCues.length; ++i) {
    const cue = activeCues[i];
    
    if (cue?.value?.key === GOOGLE_DAI_TIMED_METADATA_KEY) {
      this.streamManager.onTimedMetadata({
        [GOOGLE_DAI_TIMED_METADATA_KEY]: cue.value.data,
      }); 
    }
  } 
});

Note that value doesn't guarantee to exist on class VTTCue so we should check that. Also according to Google IMA DAI SDK doc, the metadata object should have one and only one property 'TXXX'. When I test a DAI stream, I can see that the timed metadata text track cues do have cue.value.key === 'TXXX':

[Kiro705]

Hello @alexypdu ,

Thanks for identifying this issue.

Before the commit 59c70b0, can you detail how to reproduce the issue you originally saw?

The one that had been causing the Uncaught TypeError: undefined is not iterable error. I don't think it was happening in every instance of DAI playback, but may be mistaken. This info will help me in idenitfying a fix for the original and new issue.

Thank you,
Jackson
IMA SDK team

[alexypdu]

Hello @alexypdu ,

Thanks for identifying this issue.

Before the commit 59c70b0, can you detail how to reproduce the issue you originally saw?

The one that had been causing the Uncaught TypeError: undefined is not iterable error. I don't think it was happening in every instance of DAI playback, but may be mistaken. This info will help me in idenitfying a fix for the original and new issue.

Thank you, Jackson IMA SDK team

Hello @Kiro705, so two things are happening here:

---

In videojs-ima@2.4.0 we try to iterate track.activeCues_:

videojs-ima/src/dai/sdk-impl.js

Lines 134 to 146 in 187056e

	SdkImpl.prototype.onAddTrack = function(event) {
	const track = event.track;
	if (track.kind === 'metadata') {
	track.mode = 'hidden';
	track.addEventListener('cuechange', (e) => {
	for (const cue of track.activeCues_) {
	const metadata = {};
	metadata[cue.value.key] = cue.value.data;
	this.streamManager.onTimedMetadata(metadata);
	}
	});
	}
	};

which only exists (and I believe it intends to be a private property) in Video.js TextTrack implementation. So if someone is using nativeTextTracks, then track.activeCues_ would be undefined and hence result in the error:

Uncaught TypeError: undefined is not iterable.

---

In videojs-ima@2.5.0 we try to iterate track.activeCues instead:

videojs-ima/src/dai/sdk-impl.js

Lines 135 to 147 in 0f975fa

	SdkImpl.prototype.onAddTrack = function(event) {
	const track = event.track;
	if (track.kind === 'metadata') {
	track.mode = 'hidden';
	track.addEventListener('cuechange', (e) => {
	for (const cue of track.activeCues) {
	const metadata = {};
	metadata[cue.value.key] = cue.value.data;
	this.streamManager.onTimedMetadata(metadata);
	}
	});
	}
	};

However, if someone is using Video.js emulated text tracks (i.e., nativeTextTracks is false), then track.activeCues is not iterable since Video.js's TextTrackCueList doesn't implement [Symbol.iterator], and hence result in the error:

TypeError: Invalid attempt to iterate non-iterable instance.
In order to be iterable, non-array objects must have a Symbol.iterator method.

---

Fortunately Video.js's TextTrackCueList does implement getters for length and [index]. So I believe the safe way here is to iterate track.activeCues using a regular for loop:

for (let i = 0; i < activeCues.length; ++i) {
    const cue = activeCues[i];
}

[alexypdu]

Btw it seems like the example page is broken: False alarm. It' blocked by my ad blocker.

[Kiro705]

Btw it seems like the example page is broken:

Can you open a new issue for this. It seems to be working for me, so please include steps to reproduce the error.

[alexypdu]

Please ignore it. It was my ad blocker 😅

[Kiro705]

Looking into this issue,

When testing with videojs-ima@2.5.0 which iterates track.activeCues, I see no IMA events firing, which I think is a bug:

videojs-ima/src/dai/sdk-impl.js

Lines 135 to 147 in 0f975fa

	SdkImpl.prototype.onAddTrack = function(event) {
	const track = event.track;
	if (track.kind === 'metadata') {
	track.mode = 'hidden';
	track.addEventListener('cuechange', (e) => {
	for (const cue of track.activeCues) {
	const metadata = {};
	metadata[cue.value.key] = cue.value.data;
	this.streamManager.onTimedMetadata(metadata);
	}
	});
	}
	};

When reverting this change I see IMA events fire as expected. I do not see the Uncaught TypeError: undefined is not iterable issue with the IMA sample HLS streams (link).

I do see the regular loop with activeCues approach working too, so if you can verify that this change works for your streams (using nativeTextTracks I think), this change seems reasonable:

track.addEventListener('cuechange', function (e) {
      for (let i = 0; i < track.activeCues.length; ++i) {
          const cue = track.activeCues[i];
          var metadata = {};
          metadata[cue.value.key] = cue.value.data;
          this.streamManager.onTimedMetadata(metadata);
      }
    });

Let me know. It may also be helpful to me if you can share the stream you are using to reproduce the Uncaught TypeError: undefined is not iterable error. For privacy, feel free to email the stream parameters directly at jsui@google.com.