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

Propagate bitrate from MP3 files into Format (maybe only CBR ones?) #11124

Open
chientrm opened this issue Apr 18, 2023 · 15 comments
Open

Propagate bitrate from MP3 files into Format (maybe only CBR ones?) #11124

chientrm opened this issue Apr 18, 2023 · 15 comments

Comments

@chientrm
Copy link

Bug

D/CCodecConfig(10713): no c2 equivalents for log-session-id
D/CCodecConfig(10713): no c2 equivalents for flags
D/CCodecConfig(10713): config failed => CORRUPTED
D/CCodecConfig(10713): c2 config diff is   c2::u32 raw.sample-rate.value = 48000
W/Codec2Client(10713): query -- param skipped: index = 1107298332.
D/CCodec  (10713): client requested max input size 4096, which is smaller than what component recommended (8192); overriding with component recommendation.
W/CCodec  (10713): This behavior is subject to change. It is recommended that app developers double check whether the requested max input size is in reasonable range.
D/CCodec  (10713): setup formats input: AMessage(what = 0x00000000) = {
D/CCodec  (10713):   int32_t bitrate = 64000
D/CCodec  (10713):   int32_t channel-count = 2
D/CCodec  (10713):   int32_t max-input-size = 8192
D/CCodec  (10713):   string mime = "audio/mpeg"
D/CCodec  (10713):   int32_t sample-rate = 48000
D/CCodec  (10713): }
D/CCodec  (10713): setup formats output: AMessage(what = 0x00000000) = {
D/CCodec  (10713):   int32_t channel-count = 2
D/CCodec  (10713):   string mime = "audio/raw"
D/CCodec  (10713):   int32_t sample-rate = 48000
D/CCodec  (10713): }
I/CCodecConfig(10713): query failed after returning 7 values (BAD_INDEX)
D/MediaCodec(10713): keep callback message for reclaim
W/Codec2Client(10713): query -- param skipped: index = 1342179345.
W/Codec2Client(10713): query -- param skipped: index = 2415921170.
D/C2Store (10713): Using ION
D/CCodecBufferChannel(10713): [c2.android.mp3.decoder#865] Created input block pool with allocatorID 16 => poolID 17 - OK (0)
I/CCodecBufferChannel(10713): [c2.android.mp3.decoder#865] Created output block pool with allocatorID 16 => poolID 451 - OK
D/CCodecBufferChannel(10713): [c2.android.mp3.decoder#865] Configured output block pool ids 451 => OK
E/ion     (10713): ioctl c0044901 failed with code -1: Invalid argument
D/MediaCodec(10713): keep callback message for reclaim
I/CCodecConfig(10713): query failed after returning 7 values (BAD_INDEX)
W/Codec2Client(10713): query -- param skipped: index = 1342179345.
W/Codec2Client(10713): query -- param skipped: index = 2415921170.
D/AudioTrack(10713): getTimestamp_l(6997): device stall time corrected using current time 466586944835654
D/BufferPoolAccessor2.0(10713): bufferpool2 0x7351c28368 : 5(40960 size) total buffers - 0(0 size) used buffers - 138/143 (recycle/alloc) - 5/276 (fetch/transfer)

You can clearly see this line: int32_t bitrate = 64000.

@christosts
Copy link
Contributor

Do you observe issues on the actual audio you listen to? The log you refer to is coming from the platform and is probably printing a default value because the player is not setting a bitrate for audio formats.

@christosts christosts self-assigned this Apr 18, 2023
@chientrm
Copy link
Author

chientrm commented Apr 18, 2023

I can't tell what bitrate the play is playing because audioFormat.bitrate return -1 all the time and my ears can't clearly tell it.
Meanwhile, I can make sure that the url returns a 320kbps audio file.
Also it's not HLS but single mp3 url.

@christosts
Copy link
Contributor

ExoPlayer does not alter with the media bitrate and just feeds the audio to the device. The log is coming from the platform amd the value is probably a default expected bitrate printed because the player does not define one when setting the decoder. If you prefer, can you share the mp3 file with us so that we can test too?

@chientrm
Copy link
Author

ExoPlayer does not alter with the media bitrate and just feeds the audio to the device. The log is coming from the platform amd the value is probably a default expected bitrate printed because the player does not define one when setting the decoder. If you prefer, can you share the mp3 file with us so that we can test too?

Here is the audio file: Dreamer 320kbps - Alan Walker
It is NoCopyrightSound so no worry about licensing.

The actual audio URL involves Token verification and App Check verification so I can't create an unauthorized URL for sharing. Instead, if you wouldn't bother, please take a quick look at my actual app: YouWave - Google Play. Scrolling to the bottom, you will find some NCS music tracks on the home page.

@christosts
Copy link
Contributor

I played the file, and as I described above, the player is not reading the bitrate from file header and neither sets it to MediaCodec. The file is passed straight to the device.

@chientrm
Copy link
Author

So that the audio is played at 320kbps successfully?
Meanwhile, how can I make the player to read the bitrate from the url so that I can get bitrate information in my app instead of making an extra request to my api server? 🤔

@christosts christosts reopened this Apr 18, 2023
@christosts
Copy link
Contributor

This information should be taken while parsing the file, which apparently is not picked up the specific mp3 file. There is not API at the moment. @rohitjoins do you think it's feasible to parse bitrate from the mp3 container?

@christosts christosts assigned rohitjoins and unassigned christosts Apr 18, 2023
@icbaker
Copy link
Collaborator

icbaker commented Apr 19, 2023

The provided file is constant bitrate, so the bitrate can be determined by simply doing (file.length() * 8) / (player.getDuration() / 1000).

It's not clear to me whether there's any value in ExoPlayer doing this (since an app can do it so easily themselves). We generally don't parse bitrates from progressive containers, since we only use the bitrate for selecting different tracks in adaptive media.

@chientrm
Copy link
Author

This is a very nice idea but how do I get file.length()? 🤔
Anyway I can parse Content-Length header or other custom headers? 💭

@icbaker icbaker assigned icbaker and unassigned rohitjoins Apr 19, 2023
@icbaker
Copy link
Collaborator

icbaker commented Apr 20, 2023

Ah yeah, getting the file length in a general way is hard (I was assuming you had the file locally).

I looked into modifying Mp3Extractor to set the bitrate into the Format. We already have a bitrate value available insynchronizedHeader.bitrate here:

new Format.Builder()
.setSampleMimeType(synchronizedHeader.mimeType)
.setMaxInputSize(MpegAudioUtil.MAX_FRAME_SIZE_BYTES)
.setChannelCount(synchronizedHeader.channels)
.setSampleRate(synchronizedHeader.sampleRate)
.setEncoderDelay(gaplessInfoHolder.encoderDelay)
.setEncoderPadding(gaplessInfoHolder.encoderPadding)
.setMetadata((flags & FLAG_DISABLE_ID3_METADATA) != 0 ? null : metadata)

Unfortunately that's just the bitrate of the first frame we've synchronized on, and we don't know if the file is VBR or CBR. If it's CBR we can know that every frame will have the same bitrate so it's safe/sensible to put it into the Format. But if it's VBR we can't really say anything about the bitrate of the rest of the file, so it's not safe to put it into the Format.

I tried naively checking if the file is CBR by checking seeker instanceof ConstantBitrateSeeker but this is false for CBR files with a XING header, like the one you linked in the original comment - so it's not a very good guess.

tl;dr we can keep this open as a low-priority request, but it's not clear how to easily do it in a robust way, and i'm afraid we're unlikely to work on it (since, as above, parsing bitrates from progressive containers is generally not important for ExoPlayer since we don't use the information for anything).

@icbaker icbaker changed the title Player always play 64kbps although the url return 320kbps audio Parse bitrate from MP3 files (maybe only CBR ones?) Apr 20, 2023
@icbaker icbaker changed the title Parse bitrate from MP3 files (maybe only CBR ones?) Propagate bitrate from MP3 files into Format (maybe only CBR ones?) Apr 20, 2023
@icbaker icbaker removed the question label Apr 20, 2023
@chientrm
Copy link
Author

chientrm commented Apr 21, 2023

Ah yeah, getting the file length in a general way is hard (I was assuming you had the file locally).

I looked into modifying Mp3Extractor to set the bitrate into the Format. We already have a bitrate value available insynchronizedHeader.bitrate here:

new Format.Builder()
.setSampleMimeType(synchronizedHeader.mimeType)
.setMaxInputSize(MpegAudioUtil.MAX_FRAME_SIZE_BYTES)
.setChannelCount(synchronizedHeader.channels)
.setSampleRate(synchronizedHeader.sampleRate)
.setEncoderDelay(gaplessInfoHolder.encoderDelay)
.setEncoderPadding(gaplessInfoHolder.encoderPadding)
.setMetadata((flags & FLAG_DISABLE_ID3_METADATA) != 0 ? null : metadata)

Unfortunately that's just the bitrate of the first frame we've synchronized on, and we don't know if the file is VBR or CBR. If it's CBR we can know that every frame will have the same bitrate so it's safe/sensible to put it into the Format. But if it's VBR we can't really say anything about the bitrate of the rest of the file, so it's not safe to put it into the Format.

I tried naively checking if the file is CBR by checking seeker instanceof ConstantBitrateSeeker but this is false for CBR files with a XING header, like the one you linked in the original comment - so it's not a very good guess.

tl;dr we can keep this open as a low-priority request, but it's not clear how to easily do it in a robust way, and i'm afraid we're unlikely to work on it (since, as above, parsing bitrates from progressive containers is generally not important for ExoPlayer since we don't use the information for anything).

I tried that synchronizedHeader.bitrate and it returns -1 🤔

@icbaker
Copy link
Collaborator

icbaker commented Apr 21, 2023

I see the 320kB bitrate returned from synchronizedHeader.bitrate. Concretely: adding the following Format.Builder method calls to Mp3Extractor.readInternal and then playing the file provided above logs a bitrate of 320kB when the Format is logged

.setPeakBitrate(synchronizedHeader.bitrate)
.setAverageBitrate(synchronizedHeader.bitrate)
group [                                                                                                  
  [X] Track:0, id=null, mimeType=audio/mpeg, bitrate=320000, channels=2, sample_rate=48000, supported=YES
]                                                                                                        

You're welcome to make this change in a local fork of ExoPlayer if you want to assume that you always play CBR MP3 files.

@chientrm
Copy link
Author

I see the 320kB bitrate returned from synchronizedHeader.bitrate. Concretely: adding the following Format.Builder method calls to Mp3Extractor.readInternal and then playing the file provided above logs a bitrate of 320kB when the Format is logged

.setPeakBitrate(synchronizedHeader.bitrate)
.setAverageBitrate(synchronizedHeader.bitrate)
group [                                                                                                  
 [X] Track:0, id=null, mimeType=audio/mpeg, bitrate=320000, channels=2, sample_rate=48000, supported=YES
]                                                                                                        

You're welcome to make this change in a local fork of ExoPlayer if you want to assume that you always play CBR MP3 files.

Actually, I'm using the library media3 which includes ExoPlayer inside. 🤔

@icbaker
Copy link
Collaborator

icbaker commented Apr 21, 2023

Actually, I'm using the library media3 which includes ExoPlayer inside. 🤔

Well then you filed in the wrong issue tracker (and ignored the prompt to file all issues, even ExoPlayer related ones, in https://github.com/androidx/media/issues)...

Screenshot 2023-04-21 at 09 52 00

The equivalent link for forking and depending locally is here: https://github.com/androidx/media/blob/release/README.md#locally

@Tolriq
Copy link
Contributor

Tolriq commented Apr 25, 2023

For the record here's what I use on my version that for now covers all the cases I'm aware of.

int bitrate;
long length = input.getLength();
long duration = seeker.getDurationUs();
if (seeker instanceof VbriSeeker) {
   if (length != C.LENGTH_UNSET && duration != C.TIME_UNSET && duration != 0 ) {
     bitrate = (int) ((length * 8) / (duration / 1000)) * 1000;
   } else {
     bitrate = 0;
   }
} else if (seeker instanceof XingSeeker) {
  if (length != C.LENGTH_UNSET && duration != C.TIME_UNSET && duration != 0) {
    bitrate = (int) ((length * 8) / (duration / 1000)) * 1000;
  } else {
    bitrate = 0;
  }
} else {
  bitrate = synchronizedHeader.bitrate;
}

then .setAverageBitrate(bitrate) on the builder.

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

6 participants