Specify audio codec registration priming sample handling (#944) - #949
Specify audio codec registration priming sample handling (#944)#949dalecurtis wants to merge 1 commit into
Conversation
|
This will probably continue to fail until the first PR lands. |
|
@tidoust in case there's a better way to do this. |
| {{AudioDecoder/[[priming samples to discard]]}} <em class="rfc2119">MAY</em> be | ||
| set to that number of [=priming samples=]. | ||
|
|
||
| Subsequent {{EncodedAudioChunk}}s <em class="rfc2119">MUST NOT</em> update |
There was a problem hiding this comment.
afconvert will typically use 2112 priming frames. ffmpeg's encoder, in contrast, will only have 1024.
This means that priming samples will span 3 packets (1024 + 1024 + 64) in the first case, but a single one in the second case.
There was a problem hiding this comment.
I intend that to be fine with the language here. This is just supposed to prevent cases where multiple in-band packets express individual priming samples. I.e., only the first packet should carry the priming samples. I wrote it this way to avoid dealing with uncertainties around how to combine priming across packets. E.g., Given packets p1..N, only p1 may contain priming information. Priming information on p2+ will be ignored, but the priming information on p1 may span multiple packets.
| in-band encoder delay information (for example, through an encoder | ||
| identification string in an extension payload fill element), | ||
| {{AudioDecoder/[[priming samples to discard]]}} <em class="rfc2119">MAY</em> be | ||
| set to that number of [=priming samples=]. |
There was a problem hiding this comment.
I don't know of an encoder that does this, do you? They are in elst in mp4, DiscardPadding in mkv, and never in-band.
This also means cannot be solved in e.g. adts.
There was a problem hiding this comment.
libfaac is treated as having an implicit 1024 samples by ffmpeg:
I believe that can be tagged in an ADTS buffer, but haven't checked closely. libfaac is pretty ancient at this point, so we could conceivably drop this section.
There was a problem hiding this comment.
I've never seen this in ADTS.
| 4. Expectations for {{EncodedAudioChunk}} or {{EncodedVideoChunk}} | ||
| {{EncodedVideoChunk/[[type]]}}. | ||
| 4. Where applicable, a registration specification may include a section describing [=partial dictionary|extensions to the dictionaries=] used in the `configure()`, `decode()` and `encode()` methods of the decoder and encoder interfaces (e.g., {{AudioDecoderConfig}}, {{VideoDecoderConfig}}, {{AudioEncoderConfig}}, {{VideoEncoderConfig}}, {{VideoEncoderEncodeOptions}}). | ||
| 5. Where applicable, an audio codec registration specification SHOULD describe how {{AudioDecoder}}'s {{AudioDecoder/[[priming samples to discard]]}} internal slot is initialized from the {{AudioDecoderConfig/description}} and updated from decoded chunks. If unspecified for an audio codec, the slot is initialized to `0` and not updated. |
There was a problem hiding this comment.
I think we need to make this explicit and not in the description, because there's no standard payload that is defined at the codec to do this, generally speaking. AAC's AudioSpecificConfig has no field for that, ADTS has no field, the answer is always in the container (elst in mp4, iTunSMPB, CodecDelay, etc.) or sometimes we have nothing, and that's life. Sometimes the codec/containers lines are blurry e.g. in mp3.
The goal being to be able to roundtrip audio, here are two things to consider (see #626 also):
- priming samples at the start
- padding at the end
we need to have encoder produce those values, and decoder use those values.
For this, and AudioEncoder must output a number of priming samples with its first packet in a DecoderConfig. It can do this because it knows from the start how much it needs. This depends on the codec and the implementation of that codec.
For the padding at the end, this is different: say we feed precisely one second of audio at 48kHz into an AAC encoder that does 2112 frames of priming (e.g. afconvert -s 3 -f mp4f -d aac). The total frame that will be present in the bitstream if we don't discard:
2112 + 48000 = 50112 frames needed
ceil(50112 / 1024) = 49 packets = 50176, aligned to 1024
50176 - 50112 = 64 of padding
So our decoder should know discard 2112 from the start. And when reaching the end (without reconfigure), it should trim 64 frames off the end.
We can only do this when we finalize the encoding. Thankfully it's not hard, here what we could do:
When encoding, we feed AudioData. At some point, we feed the last one, it has less frames. At this point we want to flush to get everything out, the last packet likely has a non-zero trimEnd on EncodedAudioChunk.
So: trimEnd (in frames) constructible on EncodedAudioChunkInit and readable on EncodedAudioChunk
partial interface AudioDecoderConfig {
[EnforceRange] unsigned long primingFrames = 0;
}
partial interface EncodedAudioChunk {
readonly attribute unsigned long trimEnd;
}
partial dictionnary EncodedAudioChunkInit {
[EnforceRange] unsigned long trimEnd = 0;
}I think that solves everything? In effect this also is a proposal for #626.
There was a problem hiding this comment.
I agree, but I think we should do that after this change lands. I was leaving that part to you in a follow-up per our WG discussion. The initial text here is just supposed to cover the in-band parts necessary to fix the negative timestamps issue.
Generally what you propose sounds good to me, I think we might want to work on the name trimEnd a bit since it seems to imply an action. Opus uses endTrimming . Something like trailingPrimingFrames would be symmetric with primingFrames
Update codec registry entry requirements and add Priming Samples sections for Opus, MP3, and AAC describing how [[priming samples to discard]] is initialized and updated.
This is done as a stacked PR to avoid registry bikeshed breakages due to the new internal slot definition.
See #944
See #626