On 12/02/2011 04:32 AM, Martin Storsjö wrote: > On Thu, 1 Dec 2011, Justin Ruggles wrote: > >> On 12/01/2011 06:24 PM, Martin Storsjö wrote: >> >>> Earlier, bits per sample was defined as 8, which wasn't chosen >>> with any thought, only chosen since it didn't seem to change >>> anything compared to not defining it at all. >>> >>> g722 encodes 2 samples into one byte codeword, therefore the >>> bits per sample is 4. By changing this, the generated timestamps >>> for streams encoded with g722 become correct. >> >> While technically true, this causes problems with WAV. In WAV files, the >> bits-per-sample field in the header is supposed to be 8 (or 7 or 6) not >> 4. So we need an override for g722 in the WAV header writing function in >> lavf to write the correct value. > > Yeah, I came to think of this detail right after posting the patch - I > guess I chose 8 since it was the default for the "number of bits per > codeword/sample pair", which the decoder uses - my patch also makes the > raw g722 decoder set bits_per_coded_sample to 4, which gives warnings in > the decoder, which I realized after posting it. > > As for WAV files, I don't have any samples, it'd be good to know what the > 6/7 bits per codeword samples look like, if the words are packed tightly, > or the extra bits just are to be ignored. (Currently the decoder doesn't > assume tightly packed codewords.)
Well, the only sample I have been able to find is http://samples.libav.org/A-codecs/g722/msn-messenger-g722.wav And it actually has bits-per-sample as 16. :/ I also could not find a G.722 codec for Windows to see how WMP handles it. I've never seen a 6/7 bits sample either, other than the raw ITU-T testsuite. So maybe until we actually come across such samples in a container to see how it's handled, we could just assume 4 bits per sample and provide a decoder option for code_size. >> See Bug #54 > > Oh, didn't know about this issue. > >>> Due to the way avcodec_encode_audio is defined for non frame >>> based codecs, calculating the number of input samples in the >>> encode function has to be changed once the bits per samples >>> definition is changed. >> >> >> It isn't defined that way for non-frame-based codecs, it's defined that >> way for PCM. The g722 encoder is just wrong. All other ADPCM encoders >> set frame_size and use it accordingly. > > Hmm, so are there any other non-PCM encoders that don't set a frame size? > avcodec.h says this behaviour is for PCM codecs only, while avcodec.c does > it for any codec that has a frame_size <= 1. AFAIK, PCM and G.722 are the only encoders which do not set a frame_size > 1. But I haven't checked them all yet. > My issue (that pts isn't generated properly) is fixed by setting any > frame_size in the encoder init function (and changing the encode_frame > function accordingly), but does that make sense for a codec that can use > any frame size? (For this codec, the user could set any other frame size > he wants after initializing the encoder.) And if we'd want to change this > to use a default frame size, what would be a suitable default? If the user changes frame_size after init with almost any encoder (with the exception of the last frame for CODEC_CAP_SMALL_LAST_FRAME) it will generally break horribly. I'm actually debating about how to handle PCM and other such codecs in the new encoding API that I'm working on. One option would be a CODEC_CAP that indicates that the encoder will take any number of samples from the user (via AVFrame.nb_samples) and encode them. Another option would be to have them behave like all the others and set some arbitrary frame_size. -Justin _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
