On Mon, Nov 28, 2011 at 06:12:57PM -0500, Justin Ruggles wrote:
> On 11/28/2011 05:57 PM, Janne Grunau wrote:
> 
> > On Sat, Nov 26, 2011 at 05:06:12PM -0500, Justin Ruggles wrote:
> >> Add AV_NUM_DATA_POINTERS to simplify the bump transition.
> >> This will allow for supporting more planar audio channels without having to
> >> allocate separate pointer arrays.
> >> ---
> >>  doc/APIchanges       |    4 ++++
> >>  libavcodec/avcodec.h |   11 ++++++++---
> >>  libavcodec/version.h |    6 ++++--
> >>  3 files changed, 16 insertions(+), 5 deletions(-)
> > 
> > [...]
> >> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> >> index 43abcd8..1ea96a6 100644
> >> --- a/libavcodec/avcodec.h
> >> +++ b/libavcodec/avcodec.h
> >> @@ -927,13 +927,18 @@ typedef struct AVPacket {
> >>   * sizeof(AVFrame) must not be used outside libav*.
> >>   */
> >>  typedef struct AVFrame {
> >> +#if FF_API_DATA_POINTERS
> >> +#define AV_NUM_DATA_POINTERS 4
> >> +#else
> >> +#define AV_NUM_DATA_POINTERS 8
> >> +#endif
> >>      /**
> >>       * pointer to the picture planes.
> >>       * This might be different from the first allocated byte
> >>       * - encoding:
> >>       * - decoding:
> >>       */
> >> -    uint8_t *data[4];
> >> +    uint8_t *data[AV_NUM_DATA_POINTERS];
> >>      int linesize[4];
> > 
> > keeping the four here looks inconsistent. I know that the intention is
> > to use only linesize[0] if it's the same for all planes but that should
> > be at least documented (can't remember if it's added later).
> > 
> > I would extend linesize to AV_NUM_DATA_POINTERS too.
> > 
> > Not extending it screams for bugs like the following:
> > 
> > for (i = 0; i < nb_channels; i++) {
> >     ...
> >     memcpy(dst, f.data[i], f.linesize[i]);
> >     ...
> > }
> 
> 
> a later patch that actually allows for planar audio makes it very clear
> (and even more so after local changes as recommended by luca) that only
> linesize[0] is set for audio.

yes, that is fine to avoid allocating extended_linesize too.

> as for video, we can always change that later if we need to add support
> for more than 4 planes (e.g. cmyk+alpha).

I disagree here, we can't change it easily (only with another major bump)
and it just feels wrong to have differently sized arrays for connected
fields.

Janne
> 
> -Justin
> _______________________________________________
> libav-devel mailing list
> [email protected]
> https://lists.libav.org/mailman/listinfo/libav-devel
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to