Include event ID in findSimilarRegion_() check #2077
Description
Have you read the FAQ and checked for duplicate open issues?
Yes
What version of Shaka Player are you using?
v2.5.4
Can you reproduce the issue with our latest release version?
Yes
Can you reproduce the issue with the latest code from master?
Yes
Are you using the demo app or your own custom app?
Custom app
If custom app, can you reproduce the issue using our demo app?
I did not try the demo app
What browser and OS are you using?
Chrome/MacOS
For embedded devices (smart TVs, etc.), what model and firmware version are you using?
N/A
What are the manifest and license server URIs?
Sample manifest which demonstrates the issue:
<Period id="1-340995-128166" start="PT480.04266666666666S">
<EventStream schemeIdUri="urn:mpeg:dash:event:callback:2014" value="1" timescale="90000">
<Event presentationTime="0" duration="0" id="0" messageData="-scrubbed-"></Event>
<Event presentationTime="0" duration="0" id="1" messageData="-scrubbed-"></Event>
<Event presentationTime="0" duration="1354353" id="2" messageData="-scrubbed-"></Event>
<Event presentationTime="0" duration="338588" id="3" messageData="-scrubbed-"></Event>
</EventStream>
...
What did you do?
Play a DASH stream with elements as mentioned above.
What did you expect to happen?
All events are observed via timelineregionadded event listener.
What actually happened?
Events with same presentationTime and duration but different id are currently de-duped by findSimilarRegion_(). This does not work for our use case where we are using to signal Ad beacons. There can be multiple beacons at the same presentationTime offset. The only way to differentiate them is using id.
Following patch fixes the problem. I can submit a pull request if it is acceptable.
diff --git a/lib/media/region_timeline.js b/lib/media/region_timeline.js
index 4ce44f39..35320c59 100644
--- a/lib/media/region_timeline.js
+++ b/lib/media/region_timeline.js
@@ -69,8 +69,8 @@ shaka.media.RegionTimeline = class {
}
/**
- * Find a region in the timeline that has the same scheme id uri, start time,
- * and end time. If these three parameters match, we assume it to be the same
+ * Find a region in the timeline that has the same scheme id uri, id, start time,
+ * and end time. If these four parameters match, we assume it to be the same
* region. If no similar region can be found, |null| will be returned.
*
* @param {shaka.extern.TimelineRegionInfo} region
@@ -79,9 +79,10 @@ shaka.media.RegionTimeline = class {
*/
findSimilarRegion_(region) {
for (const existing of this.regions_) {
- // The same scheme ID and time range means that it is similar-enough to
+ // The same schemeIdUri, id and time range means that it is similar-enough to
// be the same region.
const isSimilar = existing.schemeIdUri == region.schemeIdUri &&
+ existing.id == region.id &&
existing.startTime == region.startTime &&
existing.endTime == region.endTime;