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

Dev v2 dlb/oss/contribution 01 #10968

Conversation

sqshet-dlb
Copy link

No description provided.

Test: Verified playback of a DD+JOC content from ExoPlayer on a car
      emulator running Android-S.
Change-Id: I27d1f47b89b77e382cc2181273987293eaa32e9a
@sqshet-dlb sqshet-dlb changed the base branch from release-v2 to dev-v2 February 8, 2023 00:10
@tianyif tianyif self-requested a review February 9, 2023 11:54
@@ -1803,6 +1803,8 @@ public static int getAudioTrackChannelConfig(int channelCount) {
return AudioFormat.CHANNEL_OUT_5POINT1 | AudioFormat.CHANNEL_OUT_BACK_CENTER;
case 8:
return AudioFormat.CHANNEL_OUT_7POINT1_SURROUND;
case 10:
return AudioFormat.CHANNEL_OUT_5POINT1POINT4;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the commit! We have some details hope to be confirmed:

  1. Is 10 the channel count of the compressed DD+JOC stream or the decoded stream? The reason why we are asking is similar to the discussion in another pull request. At track selection, we will check if the audio can be spatialized and then select that track if possible. If 10 is the channel count of the compressed DD+JOC stream, then we will call spatializer.canBeSpatialized() against CHANNEL_OUT_5POINT1POINT4. But if the decoder outputs a different channel count that the spatializer cannot spatialize, the user won't be able to hear the spatial audio.

  2. Could you please test this change on the device on Android 12L+? ExoPlayer didn't set the key MediaFormat.KEY_MAX_OUTPUT_CHANNEL_COUNT until the API level 32, and according to the previous discussion, the DD+JOC decoder will output 2.0ch PCM in this case.

  3. Just curious, do you know if there is any compressed streams coming with CHANNEL_OUT_7POINT1POINT2, since by making this change, we will assume that 10-channel is always mapped to 5.1.4.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below is my response to your questions:

  1. Here ChannelCount 10 is the decoded stream. On a platform that supports Spatializer, the output can be spatialized otherwise no.
  2. This change has been tested on Android-13 using emulator.
  3. We are using canonical channel mask for the 10 channels as 5.1.4 similar to the 8 channels mask of 7.1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sqshet for the response!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And before I proceed with internal review of this pull request, I have one more question -

What is channel count for the original compressed stream? My concern on the other hand is that if the original channel count is different than 10, for example some other number that falls to AudioFormat.CHANNEL_INVALID mask during the track selection, then Spatializer.canBeSpatialized() can return false with that invalid channel mask (even if the device is actually supports spatialization), then track selector will try to select an alternative track instead of DD+JOC track.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tianyif ,

The input channel count refers to the number of audio objects in the input stream whereas the output channel count maps to canonical channel masks up to 12 channels. As long as canonical channel mask up to 12 channels are supported, it should be OK.

The canonical channel mask what we wanted to add here is for the 10 channels (i.e, 5.1.4).

Thanks

@tianyif tianyif self-assigned this Feb 10, 2023
@Rhenygee

This comment was marked as duplicate.

@tianyif
Copy link
Contributor

tianyif commented May 2, 2023

Sorry for the delay. We have an ongoing pull request from DTS who also adds the case of channel count = 10 to this method, which will conflict with this one, and cause AudioTrack creation failure for DTS:X P2 stream. So we may have to refactor the method getAudioTrackChannelConfig to take the mime type as the input. After that, this pull request can still go in, but with some small modifications.

@sqshet-dlb
Copy link
Author

Thanks for the update Tianyi. Let me know if you needed anything from me.

@tianyif
Copy link
Contributor

tianyif commented May 9, 2023

Hi @sqshet,

We are currently consulting with audio framework team on the mentioned conflicts with DTS above. Have you tested the above change for API <= 31? With API <= 31, would it be possible to create an AudioTrack with channel config of 5.1.4 successfully? We got a hint that the height channel masks were added in SDK 32 (e.g. CHANNEL_OUT_TOP_BACK_LEFT), so probably AudioTrack creation with 5.1.4 will also fail for Dolby when API <= 31? Could you please confirm us? We will get back to you as well when we get a more concrete answer.

Thanks!

@sqshet-dlb
Copy link
Author

Hi @tianyif ,

We are using Android 12 (API 31) for our development.
Along with this CL, we have also pushed a change to AOSP master https://android-review.googlesource.com/c/platform/system/media/+/2356183 (merged in 12th Jan 2023) which enables us in testing 5.1.4 audio track creation and testing.

Thanks
Sampath

@tianyif
Copy link
Contributor

tianyif commented Jun 1, 2023

Hi @sqshet,

We merged another pull request androidx/media#335 recently, which touches the same canonical channel count of 10 case. Could you please test the Dolby stream?

I'm closing this pull request for now, but please feel free to let us know if there is any issue introduced for Dolby. Thanks!

@tianyif tianyif closed this Jun 1, 2023
@sqshet-dlb
Copy link
Author

Thanks for following it up @tianyif.
We will take the latest ExoPlayer release and start testing our implementation.

@google google locked and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants