[FFmpeg-devel] [PATCH] avcodec: Add bits_per_raw_sample to AVCodecParameters

[James Almer]

On 4/14/2016 7:30 PM, Michael Niedermayer wrote:
> On Tue, Apr 12, 2016 at 01:25:43PM +0200, wm4 wrote:
>> On Tue, 12 Apr 2016 13:10:09 +0200
>> Michael Niedermayer <michael at niedermayer.cc> wrote:
>>
>>> On Mon, Apr 11, 2016 at 01:32:07AM +0200, Michael Niedermayer wrote:
>>>> The bits_per_raw_sample represents the number of bits of precission per sample.
>>>>
>>>> The field is added at the logical place, not at the end as the code was just
>>>> recently added
>>>>
>>>> This fixes the regression about loosing the audio sample precission information
>>>>
>>>> The change in the fate test checksum un-does the change from the merge
>>>>
>>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>>> ---
>>>>  libavcodec/avcodec.h |   17 +++++++++++++++++
>>>>  libavcodec/utils.c   |    2 ++
>>>>  tests/ref/lavf/ffm   |    2 +-
>>>>  3 files changed, 20 insertions(+), 1 deletion(-)  
>>>
>>> new version of this with improved documentation
>>> I intend to apply this soon if noone is against
>>>
>>>
>>> From e500d2222d31368b760144b1e2b5b094f73b571b Mon Sep 17 00:00:00 2001
>>> From: Michael Niedermayer <michael at niedermayer.cc>
>>> Date: Mon, 11 Apr 2016 00:52:21 +0200
>>> Subject: [PATCH] avcodec: Add bits_per_raw_sample to AVCodecParameters
>>>
>>> The bits_per_raw_sample represents the number of bits of precission per sample.
>>>
>>> The field is added at the logical place, not at the end as the code was just
>>> recently added
>>>
>>> This fixes the regression about loosing the audio sample precission information
>>>
>>> The change in the fate test checksum un-does the change from the merge
>>>
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavcodec/avcodec.h |   21 +++++++++++++++++++++
>>>  libavcodec/utils.c   |    2 ++
>>>  tests/ref/lavf/ffm   |    2 +-
>>>  3 files changed, 24 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index b3655c5..99cbf88 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -3832,9 +3832,30 @@ typedef struct AVCodecParameters {
>>>       */
>>>      int64_t bit_rate;
>>>
>>> +    /**
>>> +     * The number of bits per sample in the codedwords.
>>> +     *
>>> +     * This is basically the bitrate per sample, it is mandatory for a bunch of
>>> +     * formats to actually decode them, its the number of bits for one sample in
>>
>> "It's". Also, add more full stops instead of creating long sentences.
>>
>>> +     * the actual coded bitstream.
>>> +     *
>>> +     * This could be for example 4 for ADPCM
>>> +     * For PCM formats this matches bits_per_raw_sample
>>> +     * Can be 0
>>> +     */
>>>      int bits_per_coded_sample;
>>>
>>>      /**
>>> +     * The number of bits of precission in the samples.
>>
>> Precision. Just get a spellchecker.
>>
>>> +     *
>>> +     * These are the bits in a decoded (raw) sample.
>>
>> Maybe: "This is the number of valid bits in each output sample. If the
>> sample format has more bits, the least significant bits are additional
>> padding bits, which are always 0. Use right shifts to reduce the sample
>> to its actual size. For example, audio formats with 24 bit samples will
>> have bits_per_raw_sample set to 24, and format set to AV_SAMPLEFMT_S32.
>> To get the original sample use "(uint32_t)sample >> 8"."
>>
>> Don't know if this is good. (Also is it even correct? I forgot whether
>> audio sample data uses negative values too.) Maybe this should be added
>> to AVCodecContext too. It's a common point of confusion.
> 
> suggested changed made ad applied

Should the change made to matroskaenc be reverted then? It was changed from
using bits_per_raw_sample to bits_per_coded_sample in the codecpar commit
because the former wasn't available back then.

> 
> ill submit a seperae patch for mentioning negative values
> 
> thanks
> 
> [...]
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>