New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 660247 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Video playback that swaps audio/video track IDs error

Project Member Reported by cblume@chromium.org, Oct 28 2016

Issue description

M55 is adding MSE [1] which gives more power to JavaScript to control media. For example, a developer can enable/disable individual tracks. This is done via track IDs which can be fetched through myVideoElement.audioTracks and myVideoElement.videoTracks. These arrays hold track IDs.

The MSE spec expects these IDs to be stable.

However, if track IDs are swapped during playback then Chrome M55 errors on playback. This is apparently undocumented in the MSE spec. But it is a new error introduced in M55 that was not erroring previously. It shows up on important web properties like Twitch.

Apparently, seeking in this VOD [2] will reproduce this error.
Seeking in a different VOD [3] will not error.
A user has recorded a video of them doing this so we can see the problem [4].

[1] https://www.w3.org/TR/media-source/
[2] https://www.twitch.tv/billy1kirby/v/95921523
[3] https://www.twitch.tv/billy1kirby/v/95734270
[4] https://webmshare.com/jMWr6
 

Comment 1 by cblume@chromium.org, Oct 28 2016

Cc: cblume@chromium.org
Cc: chcunningham@chromium.org wolenetz@chromium.org
Status: Assigned (was: Untriaged)
I think I understand what's going on and indeed this is a bug in our track handling.
In the first video segment audio track id was 1 and video track was 2, in the second segment the ids have been swapped, now audio track id is 2, video track id is 1.
The current FrameProcessor::UpdateTrack won't work in this case, since it tries to update track ids one id at a time and that's problematic: it can't move the track buffer, because there is already another media track occupying that id.
I'll prepare a fix.

Comment 3 by cblume@chromium.org, Oct 28 2016

I spoke with an engineer at Twitch's video player team.

They are aware of the issue now as well.

There was an additional comment that made a lot of sense:
"In general, there could be other discontinuities that can cause track IDs to changes. For example, we may have ads or playlists that could be inserted in a stream which will have its own set of track IDs with a different set of language renditions. I know this is not covered in the spec currently, but a common scenario with a request for enhancement added to the spec, so if we are able to support that on Chrome it certainly helps"
This is actually covered by MSE spec, see https://w3c.github.io/media-source/:

The user agent must support the following:
Track IDs changing across initialization segments if the segments describes only one track of each type.

Comment 5 by cblume@chromium.org, Oct 28 2016

Oh. Does this mean Twitch doesn't need to worry then?
Yes, at least in this particular case this is our bug, not Twitch issue.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9233a906062fd461503fca84fa2223ee315bfaf

commit b9233a906062fd461503fca84fa2223ee315bfaf
Author: servolk <servolk@chromium.org>
Date: Fri Oct 28 20:20:34 2016

Fixed track id remapping in MSE FrameProcessor

Previously FrameProcessor couldn't handle cases where audio and video
track ids were swapped between the previous and current init segment,
because we tried to do an in-place update, which is impossible to do
correctly when track ids have been swapped. Consider the case where
in the first init segment A=1,V=2 and in the second segment A=2,V=1.
In such case we can't simply move let's say track A from 1 to 2,
because track id 2 is occupied by V, and vice versa.
This CL implements a more advanced logic, that will record how track
ids have changed between segments and will then update all track buffer
mappings in FrameProcessor::UpdateTrackIds at once.

BUG= 660247 

Review-Url: https://codereview.chromium.org/2460763002
Cr-Commit-Position: refs/heads/master@{#428473}

[modify] https://crrev.com/b9233a906062fd461503fca84fa2223ee315bfaf/media/filters/frame_processor.cc
[modify] https://crrev.com/b9233a906062fd461503fca84fa2223ee315bfaf/media/filters/frame_processor.h
[modify] https://crrev.com/b9233a906062fd461503fca84fa2223ee315bfaf/media/filters/source_buffer_state.cc
[modify] https://crrev.com/b9233a906062fd461503fca84fa2223ee315bfaf/media/filters/source_buffer_state_unittest.cc

Labels: Merge-Request-55
Request to merge to M55 this fix:
https://chromium.googlesource.com/chromium/src.git/+/b9233a906062fd461503fca84fa2223ee315bfaf

It's been in Canary since Friday (56.0.2904.0) and I've verified that seeking now works on the problematic stream in Twitch:
https://www.twitch.tv/billy1kirby/v/95921523

Comment 9 by dimu@chromium.org, Oct 31 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 1 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0882ff112ce36167940baf57484de9e0c03d46d9

commit 0882ff112ce36167940baf57484de9e0c03d46d9
Author: Sergey Volk <servolk@google.com>
Date: Tue Nov 01 18:08:08 2016

Fixed track id remapping in MSE FrameProcessor

Previously FrameProcessor couldn't handle cases where audio and video
track ids were swapped between the previous and current init segment,
because we tried to do an in-place update, which is impossible to do
correctly when track ids have been swapped. Consider the case where
in the first init segment A=1,V=2 and in the second segment A=2,V=1.
In such case we can't simply move let's say track A from 1 to 2,
because track id 2 is occupied by V, and vice versa.
This CL implements a more advanced logic, that will record how track
ids have changed between segments and will then update all track buffer
mappings in FrameProcessor::UpdateTrackIds at once.

BUG= 660247 

Review-Url: https://codereview.chromium.org/2460763002
Cr-Commit-Position: refs/heads/master@{#428473}
(cherry picked from commit b9233a906062fd461503fca84fa2223ee315bfaf)

Review URL: https://codereview.chromium.org/2464153002 .

Cr-Commit-Position: refs/branch-heads/2883@{#405}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/0882ff112ce36167940baf57484de9e0c03d46d9/media/filters/frame_processor.cc
[modify] https://crrev.com/0882ff112ce36167940baf57484de9e0c03d46d9/media/filters/frame_processor.h
[modify] https://crrev.com/0882ff112ce36167940baf57484de9e0c03d46d9/media/filters/source_buffer_state.cc
[modify] https://crrev.com/0882ff112ce36167940baf57484de9e0c03d46d9/media/filters/source_buffer_state_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment