On Fri, Apr 08, 2011 at 01:44:09PM +0200, Anton Khirnov wrote:
> On Thu, Apr 07, 2011 at 10:28:25AM +0200, Kostya wrote:
> > From bb18bdff543fd8c5d3e3c89350520be7932594b9 Mon Sep 17 00:00:00 2001
> > From: Kostya Shishkov <[email protected]>
> > Date: Thu, 7 Apr 2011 09:45:47 +0200
> > Subject: [PATCH 1/2] introduce side information in AVPacket
> >
> > ---
> > libavcodec/avcodec.h | 21 +++++++++++++++++++++
> > libavcodec/avpacket.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > libavcodec/version.h | 2 +-
> > 3 files changed, 63 insertions(+), 1 deletions(-)
> >
> > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> > index 95a933d..3731a6c 100644
> > --- a/libavcodec/avcodec.h
> > +++ b/libavcodec/avcodec.h
> > @@ -1054,6 +1054,14 @@ typedef struct AVPacket {
> > int64_t dts;
> > uint8_t *data;
> > int size;
> > + /**
> > + * Additional packet data that may be provided by container. If present
> > + * it should declare side data type and size.
> > + */
> > + uint8_t *side_data;
> > + int side_data_size;
> > + int side_data_type;
> > +
> > int stream_index;
> > int flags;
> > /**
> > @@ -1089,6 +1097,9 @@ typedef struct AVPacket {
> > #define PKT_FLAG_KEY AV_PKT_FLAG_KEY
> > #endif
> >
> > +#define AV_PKT_DATA_NONE 0 ///< no packet side information
>
> nit: I don't think it makes much sense to #define this.
It's just for convenience.
> > +#define AV_PKT_DATA_PAL 1 ///< packet side information contains new
> > palette
> > +
> > /**
> > * Audio Video Frame.
> > * New fields can be added to the end of FF_COMMON_FRAME with minor version
> > @@ -3190,6 +3201,16 @@ void av_shrink_packet(AVPacket *pkt, int size);
> > int av_grow_packet(AVPacket *pkt, int grow_by);
> >
> > /**
> > + * Allocate the side information of a packet.
> > + *
> > + * @param pkt packet
> > + * @param type side information type (AV_PKT_DATA_*)
> > + * @param size wanted side information size
> > + * @return 0 if OK, AVERROR_xxx otherwise
> > + */
> > +int av_packet_new_side_data(AVPacket *pkt, int type, int size);
> > +
> > +/**
> > * @warning This is a hack - the packet memory allocation stuff is broken.
> > The
> > * packet is allocated if it was not really allocated.
> > */
> > diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> > index f6aef20..2cce0f1 100644
> > --- a/libavcodec/avpacket.c
> > +++ b/libavcodec/avpacket.c
> > @@ -26,12 +26,18 @@
> > void av_destruct_packet_nofree(AVPacket *pkt)
> > {
> > pkt->data = NULL; pkt->size = 0;
> > + pkt->side_data = NULL;
> > + pkt->side_data_size = 0;
> > }
> >
> > void av_destruct_packet(AVPacket *pkt)
> > {
> > av_free(pkt->data);
> > pkt->data = NULL; pkt->size = 0;
> > + av_free(pkt->side_data);
> > + pkt->side_data = NULL;
>
> av_freep
That should belong to separate patch cleaning whole avpacket.c, I've copied
style, not written it.
> > + pkt->side_data_size = 0;
> > + pkt->side_data_type = AV_PKT_DATA_NONE;
> > }
> >
> > void av_init_packet(AVPacket *pkt)
> > @@ -44,6 +50,9 @@ void av_init_packet(AVPacket *pkt)
> > pkt->flags = 0;
> > pkt->stream_index = 0;
> > pkt->destruct= NULL;
> > + pkt->side_data = NULL;
> > + pkt->side_data_size = 0;
> > + pkt->side_data_type = AV_PKT_DATA_NONE;
> > }
> >
> > int av_new_packet(AVPacket *pkt, int size)
> > @@ -59,6 +68,9 @@ int av_new_packet(AVPacket *pkt, int size)
> > av_init_packet(pkt);
> > pkt->data = data;
> > pkt->size = size;
> > + pkt->side_data = NULL;
> > + pkt->side_data_size = 0;
> > + pkt->side_data_type = AV_PKT_DATA_NONE;
>
> that's already done in av_init_packet()
indeed, will be dropped
> > pkt->destruct = av_destruct_packet;
> > if(!data)
> > return AVERROR(ENOMEM);
> > @@ -89,6 +101,21 @@ int av_grow_packet(AVPacket *pkt, int grow_by)
> > return 0;
> > }
> >
> > +int av_packet_new_side_data(AVPacket *pkt, int type, int size)
> > +{
> > + void *new_ptr;
> > + if ((unsigned)size > INT_MAX - FF_INPUT_BUFFER_PADDING_SIZE)
> > + return -1;
>
> return AVERROR(EINVAL);
see above av_grow_packet()
> > + new_ptr = av_malloc(size + FF_INPUT_BUFFER_PADDING_SIZE);
> > + if (!new_ptr)
> > + return AVERROR(ENOMEM);
> > + pkt->side_data = new_ptr;
>
> What's the point of new_ptr, you can malloc into pkt->side_data directly
copy-paste effect, will be fixed
> > + pkt->side_data_size = size;
> > + pkt->side_data_type = type;
> > + memset(pkt->side_data + pkt->side_data_size, 0,
> > FF_INPUT_BUFFER_PADDING_SIZE);
> > + return 0;
> > +}
> > +
> > int av_dup_packet(AVPacket *pkt)
> > {
> > if (((pkt->destruct == av_destruct_packet_nofree) || (pkt->destruct ==
> > NULL)) && pkt->data) {
> > @@ -96,6 +123,9 @@ int av_dup_packet(AVPacket *pkt)
> > /* We duplicate the packet and don't forget to add the padding
> > again. */
> > if((unsigned)pkt->size > (unsigned)pkt->size +
> > FF_INPUT_BUFFER_PADDING_SIZE)
> > return AVERROR(ENOMEM);
> > + if((unsigned)pkt->side_data_size >
> > + (unsigned)pkt->side_data_size + FF_INPUT_BUFFER_PADDING_SIZE)
> > + return AVERROR(ENOMEM);
>
> AVERROR(EINVAL) would be better IMO.
Feel free to make it uniform. Personally I don't understand why different
error codes are used in similar situations, so I just copied what's already
there.
> > data = av_malloc(pkt->size + FF_INPUT_BUFFER_PADDING_SIZE);
> > if (!data) {
> > return AVERROR(ENOMEM);
> > @@ -103,6 +133,15 @@ int av_dup_packet(AVPacket *pkt)
> > memcpy(data, pkt->data, pkt->size);
> > memset(data + pkt->size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
> > pkt->data = data;
> > + if (pkt->side_data) {
> > + data = av_malloc(pkt->side_data_size +
> > FF_INPUT_BUFFER_PADDING_SIZE);
>
> Maybe i'm just ignorant about how this stuff works, but the original
> side data already contains the padding, do you have to add more here?
It does but it's not added to declared size.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
