[FFmpeg-devel] [RFC] av_get_bits_per_sample() questions

[Justin Ruggles]

On 03/13/2011 05:24 PM, Alex Converse wrote:

> On Sun, Mar 13, 2011 at 2:20 PM, Justin Ruggles
> <justin.ruggles at gmail.com> wrote:
>> On 03/13/2011 04:59 PM, Stefano Sabatini wrote:
>>
>>> Hi,
>>>
>>> just discovered that we have:
>>>
>>> int av_get_bits_per_sample(enum CodecID codec_id);
>>> and:
>>> int av_get_bits_per_sample_fmt(enum AVSampleFormat sample_fmt);
>>>
>>> av_get_bits_per_sample() return per-codec bits per sample, and is only
>>> used in pcm:
>>> ./pcm.c:48:    avctx->bits_per_coded_sample = av_get_bits_per_sample(avctx->codec->id);
>>> ./pcm.c:93:    sample_size = av_get_bits_per_sample(avctx->codec->id)/8;
>>> ./pcm.c:232:        avctx->bits_per_raw_sample = av_get_bits_per_sample(avctx->codec->id);
>>> ./pcm.c:285:    sample_size = av_get_bits_per_sample(avctx->codec_id)/8;
>>> ./pcm.c:287:    /* av_get_bits_per_sample returns 0 for CODEC_ID_PCM_DVD */
>>>
>>> Since this is a per-codec property I wonder if it would be a better
>>> idea to put the information in the codec context and drop the
>>> function.
>>
>> I agree it's ugly.
>>
>> I think it's supposed to just be a convenience function for libavformat
>> and other muxers/demuxers to be able to handle raw pcm/adpcm/dpcm more
>> easily.  I think it would be ok to redefine how bits_per_raw_sample is
>> used as an alternative.  Currently it only has meaning for encoding with
>> sample_fmt==SAMPLE_FMT_S32, but I think it would be better if that were
>> expanded to be valid for any sample format with any raw pcm-like codec
>> (ADPCM, DPCM, PCM).  Then I think we could set that in the decoders and
>> make the function return bits_per_raw_sample just for backwards
>> compatibility.
>>
>>> The second one, av_get_bits_per_sample_fmt(), is misnamed (should be
>>> av_get_bits_per_sample()), so we may change the name to
>>> av_get_bits_per_sample2() for avoiding the conflict with the name
>>> already taken.
>>
>>
>> I agree.  But as long as we're reworking it, why not
>> av_get_bytes_per_sample() so it doesn't have to be divided by 8 everywhere?
>>
> 
> Don't some ADPCM codecs have less than 8 bits per sample.


That part is referring to the function that takes a sample format, not a
codec id.

-Justin