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

[wm4]

On Fri, 15 Apr 2016 00:06:27 -0300
James Almer <jamrial at gmail.com> wrote:

> 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.
> 

Yep.