fix: use audio offset for id3 with audio-only

[mister-ben]

Description

id3 cues are not added correctly to audio-only streams, as the test for an audio-only stream is broken. videoTimestampOffset() will never be null as it defaults to 0, so the audio offset is not used.

Specific Changes proposed

Uses the videoTimestampOffset() when segmentInfo.trackInfo.hasVideo is false rather than when videoTimestampOffset() is null

Fixes #130

Requirements Checklist

• Feature implemented / Bug fixed
• If necessary, more likely in a feature request than a bug fix
  • Unit Tests updated or fixed
  • Example created (below)
• Reviewed by Two Core Contributors

Example

• Open the deploy preview and https://videojs-http-streaming.netlify.app to compare.
• In each console, add

  const printCueData = (cues) => {
    Array.from(cues).forEach((cue) => {
      if ("value" in cue && cue?.value?.data) {
        const value = new TextDecoder()
          .decode(cue.value.data)
          .replace("\x03", "")
          .replace("\x00", "");
        console.log({
          value,
          start: cue.startTime,
          end: cue.endTime,
        });
      }
    });
  };
  
  const onAddTrack = ({ track }) => {
    if (
      !(track?.kind === "metadata" && track.label === "Timed Metadata")
    ) {
      return;
    }
  
    const onCueChange = () => {
      if (!track.activeCues) {
        return;
      }
  
      const cues = printCueData(track.activeCues);
    };
  
    track.mode = "hidden";
    track.addEventListener("cuechange", onCueChange);
  };
  
  player.textTracks().addEventListener("addtrack", onAddTrack);
  

• In each load https://cue-points-test.vercel.app/audio/master.m3u8 and play
• Observe cues logged to the console in deploy preview (like {value: '{"webcastId":"635fc...)

Thanks to @berrutti for the test streams from #130

[codecov]

Codecov Report

Merging #1386 (1a9c09f) into main (b4f44e4) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1386   +/-   ##
=======================================
  Coverage   85.40%   85.40%           
=======================================
  Files          40       40           
  Lines        9963     9963           
  Branches     2308     2308           
=======================================
  Hits         8509     8509           
  Misses       1454     1454           

Impacted Files	Coverage Δ
src/playlist-controller.js	95.16% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[berrutti]

Hey @mister-ben, thanks a lot. This is great. Have a great weekend.

[adrums86]

Good fix. Thanks for this @mister-ben

[adrums86]

My apologies @mister-ben, moving the common metadata functionality up to accommodate EventStream messed up your fix. The timestamp offset ternary now lives in

http-streaming/src/playlist-controller.js

Line 2015 in f6b9498

const timestampOffset = this.sourceUpdater_.videoTimestampOffset() === null ?

I think it could be done without the segmentData like this: const timestampOffset = this.sourceUpdater_.videoBuffer ?

[mister-ben]

I think it could be done without the segmentData like this: const timestampOffset = this.sourceUpdater_.videoBuffer ?

That works with the test stream, but there's a videoBuffer present in the tests...

[adrums86]