Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Audio tracks(and probably text tracks) do not read labels from manifest #6960

Open
RomeroDiver opened this issue Nov 24, 2020 · 10 comments
Open

Comments

@RomeroDiver
Copy link

RomeroDiver commented Nov 24, 2020

Description

When loading manifest .mpd with the

<AdaptationSet mimeType="audio/mp4" lang="und" segmentAlignment="0">
  <ContentProtection cenc:default_KID="id" schemeIdUri="urn:mpeg:dash:mp4protection:2011" value="cenc"/>
  <ContentProtection cenc:default_KID="id" schemeIdUri="urn:uuid:id" value="2.0">
    <mspr:pro>hash</mspr:pro>
    <cenc:pssh>hash</cenc:pssh>
  </ContentProtection>
  <ContentProtection cenc:default_KID="id" schemeIdUri="urn:uuid:id">
    <cenc:pssh>hash</cenc:pssh>
  </ContentProtection>
  <SegmentTemplate timescale="24000" media="audio_$Number%09d$.mp4" initialization="audio_init.mp4" duration="720720" startNumber="1"/>
  <Label>English</Label>
  <Representation id="9" bandwidth="64000" audioSamplingRate="48000" codecs="mp4a.40.5"/>
</AdaptationSet>

the player.audioTracks() return audio tracks without the label from the above manifest.

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

  1. Load a player with custom audio tracks
  2. The player will load .mpd manifest with the Label element for the audio tracks

Results

The label in the .audioTracks()[0] is actually from the lang attribute in the manifest

Expected

The label in the .audioTracks()[0] is actually from the Label element in the manifest. I've tried the label attribute on the element AdaptationSet but that didn't work as well.

Actual

The label in the .audioTracks().[0] is actually from the lang attribute in the manifest

Error output

There are no errors in the console, and the actual manifest is correct as I've linted it

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

7.8.3

browsers

Both Chrome and Firefox, I suppose the same goes for the rest of them as well

OSes

Tested on Windows 10

plugins

    "videojs-contrib-eme": "^3.7.0",
    "videojs-contrib-quality-levels": "^2.0.9",
    "videojs-hls-quality-selector": "1.1.1",
    "videojs-http-source-selector": "^1.1.6",
    "videojs-markers": "^1.0.1",
    "videojs-seek-buttons": "^1.6.0",
    "videojs-sprite-thumbnails": "^0.5.3"

Probably a duplicate of videojs/http-streaming#645

@welcome
Copy link

welcome bot commented Nov 24, 2020

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.
To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@brandonocasey
Copy link
Contributor

Hey @RomeroDiver we don't currently support <Label> in dash manifests as you reported, but we probably should. We might not have time to get to this for awhile. If you are up for making the change I can outline the process how I see it right now. First we would have to add something to know about the <Label> element. I think that would be done here:

https://github.com/videojs/mpd-parser/blob/7bf58e93a9b429107137611220e63d1ced3486c1/src/inheritAttributes.js#L224-L248
and would look something like:

const label = const role = findChildren(adaptationSet, 'Label')[0];

if (label) {
	// add the label to the representation object.
}

Then you will have to prioritze the label property here if it exists.
https://github.com/videojs/mpd-parser/blob/7bf58e93a9b429107137611220e63d1ced3486c1/src/toM3u8.js#L134-L144

@stale
Copy link

stale bot commented Jan 23, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the outdated Things closed automatically by stalebot label Jan 23, 2021
@RomeroDiver
Copy link
Author

Not outdated - just waiting for a code review :)

@stale stale bot removed the outdated Things closed automatically by stalebot label Jan 25, 2021
@gkatsev
Copy link
Member

gkatsev commented Feb 17, 2021

FYI, we haven't forgotten! Just keep getting distracted by other things!

@RomeroDiver
Copy link
Author

Hey, I've seen that the PR has been merged in the mpd-parser package, however what is the flow to push it through other dependencies cause the mpd-parser is not the direct dependency of the videojs but of the other packages? As far as I can see, the http-stream is the one to update.

@gkatsev
Copy link
Member

gkatsev commented Mar 30, 2021

mpd-parser needs to get updated in VHS and then Video.js needs to get that version of VHS.
It will likely happen either this week or next week.

@RomeroDiver
Copy link
Author

Hey, the issue still exists - now though only for textTracks, the audioTracks seem to be working properly

@RomeroDiver
Copy link
Author

Here is the PR for this issue: videojs/mpd-parser#161

@ghost
Copy link

ghost commented Aug 12, 2022

@RomeroDiver I had the same problem and opened an issue here: #7870

As you already pointed out, it's working for Audio-Tracks but not for Subtitle-Tracks like webvtt and such, I can absolutely confirm this. As a hot-fix I set the lang attribute for text tracks like that: lang="Deutsch"
where on the other side I use Lable>Deutsch</Label and lang="de" on Audio Tracks. Anyway, I would not take such a hot-fix into production as it will get broken as soon as VideoJS implements Label for Subtitle tracks ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants