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.
as for video, we can always change that later if we need to add support
for more than 4 planes (e.g. cmyk+alpha).
-Justin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
