The Wayback Machine - https://web.archive.org/web/20201105145500/https://github.com/google/ExoPlayer/pull/884
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

Added id property to the MediaFormat class #884

Merged
merged 7 commits into from Oct 27, 2015

Conversation

@RikHeijdens
Copy link
Contributor

@RikHeijdens RikHeijdens commented Oct 20, 2015

In our application we give people control of the "selected" track on the player for DASH streams however there was no way for people to identify the track, unless if you knew the bitrate or width/height properties. This change adds the id property to the MediaFormat class which is read from the DASH Media Presentation Description. This way it's easier for users to identify tracks on the player.

CLA should be signed: https://groups.google.com/forum/#!forum/jwplayer-google-contributors

Feedback is appreciated, feel free to ask questions.
Thanks.

RikHeijdens added 2 commits Oct 20, 2015
Added the property 'id' to the MediaFormat class
 which serves as an identifier for the track.

DASH Representations will have the "id's" from their
Media Presentation Description mapped to the id property
 in the MediaFormat class that will represent the track.

We needed this for an use case where we wanted to read the 'id'
value from the DASH representation and present it to the user
in order for the user to select the right track.
@RikHeijdens RikHeijdens changed the title Added id property to MediaFormat class Added id property to the MediaFormat class Oct 20, 2015
RikHeijdens added 2 commits Oct 20, 2015
…d-dash

Syncing my fork
@jeoliva

This comment has been minimized.

Copy link

@jeoliva jeoliva commented on 42351d3 Oct 20, 2015

In some way this is similar to the trackId field add in decb7f5 although you are exposing DASH representation id instead of a numeric identifier. Out of curiosity, what is the use case behind showing the DASH representation id directly to the user? Are you "hand-making" this id's to contain information (resolution, bitrate, etc) that could be useful for the user?

This comment has been minimized.

Copy link
Owner Author

@RikHeijdens RikHeijdens replied Oct 20, 2015

We provide the users of our product the ability to load any DASH stream on the player, and in many cases users load DASH streams with multiple video representations (tracks). We saw a case where a user would load a stream with various video representations that had the same height and width but a different bitrate. In our UI there was no way to differentiate the streams from each other, except for selecting them on the player and trying to figure out the visual difference. With this change users can specify how the tracks in our UI should be displayed by setting the id field in the DASH MPD.

I also noticed that id is somewhat similiar to trackId however, that identifier is numeric where as id in a DASH stream could be a string.

This comment has been minimized.

Copy link
Owner Author

@RikHeijdens RikHeijdens replied Oct 20, 2015

I created a screenshot using the ExoPlayer demo app to demonstrate the functionality.
Under the DebugTextView I added another TextView showing the 'id' of the current tracks:

device-2015-10-20-151435_small

DASH MPD:

...
    <AdaptationSet group="1" lang="en_stereo" mimeType="audio/mp4" minBandwidth="130358" maxBandwidth="130358" segmentAlignment="true">
      <Representation id="128kbps en" bandwidth="130358" codecs="mp4a.40.2" audioSamplingRate="48000">
        <AudioChannelConfiguration schemeIdUri="urn:mpeg:dash:23003:3:audio_channel_configuration:2011" value="2"/>
        <SegmentTemplate timescale="48000" duration="95232" media="audio/stereo/en/128kbit/segment_$Number$.m4s" initialization="audio/stereo/en/128kbit/init.mp4" startNumber="1"/>
      </Representation>
    </AdaptationSet>
    <AdaptationSet group="2" lang="no-voices_stereo" mimeType="audio/mp4" minBandwidth="130395" maxBandwidth="130395" segmentAlignment="true">
      <Representation id="128kbps karaoke" bandwidth="130395" codecs="mp4a.40.2" audioSamplingRate="48000">
        <AudioChannelConfiguration schemeIdUri="urn:mpeg:dash:23003:3:audio_channel_configuration:2011" value="2"/>
        <SegmentTemplate timescale="48000" duration="95232" media="audio/stereo/none/128kbit/segment_$Number$.m4s" initialization="audio/stereo/none/128kbit/init.mp4" startNumber="1"/>
      </Representation>
    </AdaptationSet>
    <AdaptationSet group="3" lang="en_surround" mimeType="audio/mp4" minBandwidth="321836" maxBandwidth="321836" segmentAlignment="true">
      <Representation id="320kbps en" bandwidth="321836" codecs="mp4a.40.5" audioSamplingRate="48000">
        <AudioChannelConfiguration schemeIdUri="urn:mpeg:dash:23003:3:audio_channel_configuration:2011" value="6"/>
        <SegmentTemplate timescale="48000" duration="95232" media="audio/surround/en/320kbit/segment_$Number$.m4s" initialization="audio/surround/en/320kbit/init.mp4" startNumber="1"/>
      </Representation>
    </AdaptationSet>
    <AdaptationSet group="4" mimeType="video/mp4" par="4096:1744" minBandwidth="258157" maxBandwidth="10285391" minWidth="422" maxWidth="4096" minHeight="180" maxHeight="1744" segmentAlignment="true" startWithSAP="1">
      <Representation id="240p 250kbps" frameRate="24" bandwidth="258157" codecs="avc1.4d400d" width="422" height="180">
        <SegmentTemplate timescale="1000" duration="2000" media="video/250kbit/segment_$Number$.m4s" initialization="video/250kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="480p 500kbps" frameRate="24" bandwidth="520929" codecs="avc1.4d4015" width="638" height="272">
        <SegmentTemplate timescale="1000" duration="2000" media="video/500kbit/segment_$Number$.m4s" initialization="video/500kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="480p 800kbps" frameRate="24" bandwidth="831270" codecs="avc1.4d4015" width="638" height="272">
        <SegmentTemplate timescale="1000" duration="2000" media="video/800kbit/segment_$Number$.m4s" initialization="video/800kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="576p 1100kbps" frameRate="24" bandwidth="1144430" codecs="avc1.4d401f" width="958" height="408">
        <SegmentTemplate timescale="1000" duration="2000" media="video/1100kbit/segment_$Number$.m4s" initialization="video/1100kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="720p 1500kbps" frameRate="24" bandwidth="1558322" codecs="avc1.4d401f" width="1277" height="544">
        <SegmentTemplate timescale="1000" duration="2000" media="video/1500kbit/segment_$Number$.m4s" initialization="video/1500kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="720p 2400kbps" frameRate="24" bandwidth="2487897" codecs="avc1.4d401f" width="1277" height="544">
        <SegmentTemplate timescale="1000" duration="2000" media="video/2400kbit/segment_$Number$.m4s" initialization="video/2400kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="1080p 3000kbps" frameRate="24" bandwidth="3113198" codecs="avc1.4d4028" width="1921" height="818">
        <SegmentTemplate timescale="1000" duration="2000" media="video/3000kbit/segment_$Number$.m4s" initialization="video/3000kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="1080p 4000kbps" frameRate="24" bandwidth="4149264" codecs="avc1.4d4028" width="1921" height="818">
        <SegmentTemplate timescale="1000" duration="2000" media="video/4000kbit/segment_$Number$.m4s" initialization="video/4000kbit/init.mp4" startNumber="1"/>
      </Representation>
      <Representation id="1080p 6000kbps" frameRate="24" bandwidth="6214307" codecs="avc1.4d4028" width="1921" height="818">
        <SegmentTemplate timescale="1000" duration="2000" media="video/6000kbit/segment_$Number$.m4s" initialization="video/6000kbit/init.mp4" startNumber="1"/>
      </Representation>
    </AdaptationSet>
    <AdaptationSet group="14" mimeType="text/vtt" lang="en">
      <Representation id="caption_en" bandwidth="256">
        <BaseURL>subtitles/subtitles_en.vtt</BaseURL>
      </Representation>
    </AdaptationSet>
...

This comment has been minimized.

Copy link

@jeoliva jeoliva replied Oct 20, 2015

Ok, thank you very much for the detailed explanation.

@ojw28
Copy link
Contributor

@ojw28 ojw28 commented Oct 23, 2015

Hey. Thanks for the contribution. I'm a little bit confused by this comment:

We saw a case where a user would load a stream with various video representations that had the same height and width but a different bitrate. In our UI there was no way to differentiate the streams from each other.

It's unclear why you need the identifier to resolve this issue. The ExoPlayer.getTrackFormat method gives you the MediaFormat of the track, which includes the bitrate. So why not just look at the bitrate field to solve this case?

The above question aside, I do think it's a good idea to propagate the identifier, and can see cases where it would be useful.

We'll try and get this merged soon, or an equivalent change in place. One thing we're mulling over is whether a separate field is the best option, or whether we should just convert the existing field to a string. I think we're thinking that converting the existing field is probably the best option.

/**
* An identifier for the format.
*/
public final String id;

This comment has been minimized.

@ojw28

ojw28 Oct 23, 2015
Contributor

As per comment, it seems better just to convert trackId into a string?

rotationDegrees, pixelWidthHeightRatio, channelCount, sampleRate, language,
subsampleOffsetUs, initializationData, adaptive, maxWidth, maxHeight);
}

public MediaFormat copyAsAdaptive() {
return new MediaFormat(trackId, mimeType, NO_VALUE, NO_VALUE, durationUs, NO_VALUE, NO_VALUE,
return new MediaFormat(id, trackId, mimeType, NO_VALUE, NO_VALUE, durationUs, NO_VALUE, NO_VALUE,

This comment has been minimized.

@ojw28

ojw28 Oct 23, 2015
Contributor

This is problematic, because you're creating an adaptive MediaFormat (i.e. one that really spans multiple individual formats) from a non-adaptive one (i.e. an arbitrary one of the individual formats), and the adaptive MediaFormat is getting the id from the arbitrary individual format that you use.

You should probably take a look at the places that call this method, and either decide to set the id to null here or allow a different id to be passed in as an argument, so you can set it to some other value, depending on what you think makes most sense.

@ojw28
Copy link
Contributor

@ojw28 ojw28 commented Oct 23, 2015

Looks pretty good. If you convert the existing id field to a string and fix the id when creating adaptive formats to do something sensible, then we'll merge this. Thanks!

@RikHeijdens
Copy link
Contributor Author

@RikHeijdens RikHeijdens commented Oct 23, 2015

It's unclear why you need the identifier to resolve this issue. The ExoPlayer.getTrackFormat method gives you the MediaFormat of the track, which includes the bitrate. So why not just look at the bitrate field to solve this case?

We would like to give users the possibility to use a 'custom' name for their tracks instead of using a predfined format. We figured out we could make this possible by using the id field in DASH streams and the NAME field in HLS streams. See #885 for the HLS variant of our implementation for this.

I will convert the existing field and fix the id when creating adaptive formats.

By the way: I was just told we had some technical problems signing the corporate CLA, one of my supervisors was not able to sign it through the webpage. He contacted Google and he's working on it.

@@ -45,20 +45,20 @@ public void testConversionToFrameworkFormat() {
initData.add(initData2);

testConversionToFrameworkFormatV16(MediaFormat.createVideoFormat(
MediaFormat.NO_VALUE, "video/xyz", 5000, 102400, 1000L, 1280, 720, initData));
null, MediaFormat.NO_VALUE, "video/xyz", 5000, 102400, 1000L, 1280, 720, initData));

This comment has been minimized.

@ojw28

ojw28 Oct 26, 2015
Contributor

Please put the indentation back to what it used to be like (4 chars) (ditto x2 below).

@@ -38,12 +38,11 @@
* the timestamps of their parent samples.
*/
public static final long OFFSET_SAMPLE_RELATIVE = Long.MAX_VALUE;

This comment has been minimized.

@ojw28

ojw28 Oct 26, 2015
Contributor

Add blank line back again?

@@ -846,6 +846,7 @@ public void run() {

public ExposedTrack(MediaFormat trackFormat, int adaptationSetIndex, Format fixedFormat) {
this.trackFormat = trackFormat;

This comment has been minimized.

@ojw28

ojw28 Oct 26, 2015
Contributor

Rm additional blank line.


}
}

This comment has been minimized.

@ojw28

ojw28 Oct 26, 2015
Contributor

Revert bad style change.

@ojw28
Copy link
Contributor

@ojw28 ojw28 commented Oct 26, 2015

Happy to merge this once stylistic alterations are reverted. I don't think the CLA has been signed yet. Any update? Thanks!

RikHeijdens added 2 commits Oct 26, 2015
@RikHeijdens
Copy link
Contributor Author

@RikHeijdens RikHeijdens commented Oct 26, 2015

I have no idea why Android Studio screwed up the code identation, it should detect existing file identation and use that for editing.

The person responsible for signing the CLA is out of office today. I'll try to update you on the CLA tomorrow.

Thanks.

@ojw28 ojw28 force-pushed the google:dev branch to 6fb5052 Oct 26, 2015
@ojw28
Copy link
Contributor

@ojw28 ojw28 commented Oct 27, 2015

CLA looks good now. Feel free to resolve the merge conflict and update if you have a chance. Otherwise I can probably do this myself tomorrow. Thanks!

@ojw28 ojw28 added the cla: yes label Oct 27, 2015
@ojw28 ojw28 merged commit ec65fe9 into google:dev Oct 27, 2015
ojw28 added a commit that referenced this pull request Oct 27, 2015
ojw28 added a commit that referenced this pull request Oct 27, 2015
@google google locked and limited conversation to collaborators Jun 28, 2017
@ojw28 ojw28 added the should merge label Aug 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.