[FFmpeg-devel] [PATCH] lavu/samplefmt: make av_samples_fill_arrays() return the required size of the buffer

[Michael Niedermayer]

On Mon, Oct 29, 2012 at 03:47:36PM +0100, Stefano Sabatini wrote:
> On date Sunday 2012-10-21 18:27:05 +0200, Michael Niedermayer encoded:
> > On Sun, Oct 21, 2012 at 11:44:59AM +0200, Stefano Sabatini wrote:
> > > On date Saturday 2012-09-08 01:20:48 +0200, Stefano Sabatini encoded:
> > > > This is technically an API break, since documentation was stating that 0
> > > > was the return error code. In practice, users will just check on ret < 0,
> > > > so it *might* be a non-issue.
> > > > 
> > > > The value is already computed in the function, so returning it comes at
> > > > no cost, and provides more information than just returning 0.
> > > > ---
> > > >  libavutil/samplefmt.c |    2 +-
> > > >  libavutil/samplefmt.h |    3 ++-
> > > >  2 files changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/libavutil/samplefmt.c b/libavutil/samplefmt.c
> > > > index a03648e..9da04bd 100644
> > > > --- a/libavutil/samplefmt.c
> > > > +++ b/libavutil/samplefmt.c
> > > > @@ -171,7 +171,7 @@ int av_samples_fill_arrays(uint8_t **audio_data, int *linesize,
> > > >      if (linesize)
> > > >          *linesize = line_size;
> > > >  
> > > > -    return 0;
> > > > +    return buf_size;
> > > >  }
> > > >  
> > > >  int av_samples_alloc(uint8_t **audio_data, int *linesize, int nb_channels,
> > > > diff --git a/libavutil/samplefmt.h b/libavutil/samplefmt.h
> > > > index 681e521..299200d 100644
> > > > --- a/libavutil/samplefmt.h
> > > > +++ b/libavutil/samplefmt.h
> > > > @@ -179,7 +179,8 @@ int av_samples_get_buffer_size(int *linesize, int nb_channels, int nb_samples,
> > > >   * @param nb_samples       the number of samples in a single channel
> > > >   * @param sample_fmt       the sample format
> > > >   * @param align            buffer size alignment (0 = default, 1 = no alignment)
> > > > - * @return                 0 on success or a negative error code on failure
> > > > + * @return                 the size in bytes required for buf, a negative error code
> > > > + *                         in case of failure
> > > >   */
> > > >  int av_samples_fill_arrays(uint8_t **audio_data, int *linesize,
> > > >                             const uint8_t *buf,
> > > 
> > > Ping.
> > 
> > should be ok
> 
> After more testing, it resulted there were some failures resulting
> from the propagation of some error codes/checks. I updated the patch
> accordingly. I suppose I could still split the patches into pieces, if
> there is a preference for that. 
> -- 
> FFmpeg = Frenzy and Fundamentalist Multipurpose Philosophical Ecumenical Genius

>  libavcodec/avcodec.h  |    3 ++-
>  libavcodec/utils.c    |    4 ++--
>  libavutil/samplefmt.c |    4 ++--
>  libavutil/samplefmt.h |    6 ++++--
>  4 files changed, 10 insertions(+), 7 deletions(-)
> 077651e39ec1ecda2cdb2ab24e9ba902fd7ee623  0001-lavu-lavc-make-samples-buffer-creation-function-retu.patch
> From cae570dab674ba12e1c8e09df2a38bd9b2a84af0 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Sat, 8 Sep 2012 00:57:18 +0200
> Subject: [PATCH] lavu/lavc: make samples buffer creation function return size
>  of the allocated buffer
> 
> This is technically an API break, since documentation was stating that 0
> was the return error code. In practice, users will just check on ret < 0,
> so it *might* be a non-issue.
> 
> The value is already computed in the function, so returning it comes at
> no cost, and provides more information than just returning 0.
> 
> The change in performed in the functions:
> libavcodec/avcodec.h:avcodec_fill_audio_frame
> libavutil/samplefmt.h:av_samples_fill_arrays
> libavutil/samplefmt.h:av_samples_alloc
> 
> and must be done at once, in order to allow correct propagation of the
> error code.
[...]

> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index 2029b53..cc983c3 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -419,7 +419,7 @@ static int audio_get_buffer(AVCodecContext *avctx, AVFrame *frame)
>          }
>          if ((ret = avcodec_fill_audio_frame(frame, avctx->channels,
>                                              avctx->sample_fmt, buf->data[0],
> -                                            buf->audio_data_size, 0)))
> +                                            buf->audio_data_size, 0)) < 0)
>              return ret;
>  
>          if (frame->extended_data == frame->data)
> @@ -1373,7 +1373,7 @@ int attribute_align_arg avcodec_encode_audio(AVCodecContext *avctx,
>          if ((ret = avcodec_fill_audio_frame(frame, avctx->channels,
>                                              avctx->sample_fmt,
>                                              (const uint8_t *)samples,
> -                                            samples_size, 1)))
> +                                            samples_size, 1)) < 0)
>              return ret;
>  
>          /* fabricate frame pts from sample count.

above 2 hunks are ok
rest seems problematic ABI/API wise

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121125/6412015b/attachment.asc>