On Wed, 14 May 2014 15:56:17 +0200, Alessandro Ghedini <[email protected]>
wrote:
> On lun, mag 12, 2014 at 07:31:13 +0200, Anton Khirnov wrote:
> >
> > On Wed, 7 May 2014 00:37:41 +0200, Alessandro Ghedini
> > <[email protected]> wrote:
> > > Hi,
> > >
> > > I've been working on merging the GIF encoder from ffmpeg, since libav's
> > > doesn't
> > > seem to work very well (in my tests anyway), and my initial approach has
> > > been to
> > > simply copy/paste lavc/gif.{c,h} and lavf/gif.c, make them compile and
> > > see how
> > > it goes (I'm using libav.git).
> > >
> > > While I've been able to compile the whole thing (with a little bit of
> > > work, due
> > > to missing av_copy_packet() and ff_alloc_packet2()), I'm still having
> > > problems
> > > apparently related to pixel formats.
> > >
> > > Basically, when I'm using ffmpeg (ffmpeg -i test.mkv test.ffmpeg.gif) it
> > > selects
> > > "bgr8" and everything goes fine.
> > >
> >
> > I think using av_packet_ref() really should work, there seems to be no
> > reason to
> > copy it. Can you post the code that fails somewhere?
>
> So, av_copy_packet() looks like this:
>
> *dst = *src;
> return copy_packet_data(dst, src, 0);
>
> where copy_data_packet() does the av_buffer_ref() thing, and copies the side
> data. So I simply replaced copy_packet_data() with av_packet_ref() and then
> used
> av_packet_unref() instead of av_free_packet()/av_freep(), like this:
>
> diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
> index c3787e1..c4a0d32 100644
> --- a/libavcodec/avpacket.c
> +++ b/libavcodec/avpacket.c
> @@ -262,7 +262,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> int av_copy_packet(AVPacket *dst, const AVPacket *src)
> {
> *dst = *src;
> - return copy_packet_data(dst, src, 0);
> + return av_packet_ref(dst, src);
> }
>
> void av_packet_free_side_data(AVPacket *pkt)
> diff --git a/libavformat/gif.c b/libavformat/gif.c
> index e817121..4230756 100644
> --- a/libavformat/gif.c
> +++ b/libavformat/gif.c
> @@ -165,7 +165,7 @@ static int flush_packet(AVFormatContext *s, AVPacket *new)
>
> avio_write(pb, pkt->data, pkt->size);
>
> - av_free_packet(gif->prev_pkt);
> + av_packet_unref(gif->prev_pkt);
> if (new)
> av_copy_packet(gif->prev_pkt, new);
>
> @@ -191,7 +191,7 @@ static int gif_write_trailer(AVFormatContext *s)
> AVIOContext *pb = s->pb;
>
> flush_packet(s, NULL);
> - av_freep(&gif->prev_pkt);
> + av_packet_unref(gif->prev_pkt);
> avio_w8(pb, 0x3b);
>
> return 0;
>
> But it spits out a whole binch of:
>
> *** Error in `devel/libav/avconv': free(): invalid pointer:
> 0x000000000a279bd0 ***
> *** Error in `devel/libav/avconv': corrupted double-linked list:
> 0x0000000009c715c0 ***
>
> and then it segfaults. The only difference I can see between
> copy_packet_data()
> and av_packet_ref() is the way side data is copied: the former uses ffmpeg's
> av_copy_packet_side_data() while the latter uses av_packet_copy_props(). So I
> replaced the av_packet_copy_props() call with the ffmpeg thing like this:
>
Aren't you confusing the AVPacket itself (which is a container for data) with
the data it contains? The two must be allocated and freed separately, and
av_packet_ref/unref only handles the data.
Check out the attached patch, it seems to work for me.
--
Anton Khirnov
>From 58739aa0f0f32cc313aca3ef1bf2be8c959af6f8 Mon Sep 17 00:00:00 2001
From: Anton Khirnov <[email protected]>
Date: Sat, 17 May 2014 17:30:09 +0200
Subject: [PATCH] gif: use av_packet_ref()
---
libavformat/gif.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/libavformat/gif.c b/libavformat/gif.c
index 4230756..13dcba3 100644
--- a/libavformat/gif.c
+++ b/libavformat/gif.c
@@ -81,7 +81,8 @@ typedef struct {
AVClass *class;
int loop;
int last_delay;
- AVPacket *prev_pkt;
+ AVPacket prev_pkt;
+ int got_first_packet;
int duration;
} GIFContext;
@@ -123,9 +124,9 @@ static int flush_packet(AVFormatContext *s, AVPacket *new)
AVIOContext *pb = s->pb;
uint8_t flags = 0x4, transparent_color_index = 0x1f;
const uint32_t *palette;
- AVPacket *pkt = gif->prev_pkt;
+ AVPacket *pkt = &gif->prev_pkt;
- if (!pkt)
+ if (!pkt->size)
return 0;
/* Mark one colour as transparent if the input palette contains at least
@@ -150,7 +151,7 @@ static int flush_packet(AVFormatContext *s, AVPacket *new)
}
if (new && new->pts != AV_NOPTS_VALUE)
- gif->duration = av_clip_uint16(new->pts - gif->prev_pkt->pts);
+ gif->duration = av_clip_uint16(new->pts - gif->prev_pkt.pts);
else if (!new && gif->last_delay >= 0)
gif->duration = gif->last_delay;
@@ -165,9 +166,9 @@ static int flush_packet(AVFormatContext *s, AVPacket *new)
avio_write(pb, pkt->data, pkt->size);
- av_packet_unref(gif->prev_pkt);
+ av_packet_unref(&gif->prev_pkt);
if (new)
- av_copy_packet(gif->prev_pkt, new);
+ av_packet_ref(&gif->prev_pkt, new);
return 0;
}
@@ -176,11 +177,13 @@ static int gif_write_packet(AVFormatContext *s, AVPacket *pkt)
{
GIFContext *gif = s->priv_data;
- if (!gif->prev_pkt) {
- gif->prev_pkt = av_malloc(sizeof(*gif->prev_pkt));
- if (!gif->prev_pkt)
- return AVERROR(ENOMEM);
- return av_copy_packet(gif->prev_pkt, pkt);
+ if (!gif->got_first_packet) {
+ int ret = av_packet_ref(&gif->prev_pkt, pkt);
+ if (ret < 0)
+ return ret;
+
+ gif->got_first_packet = 1;
+ return 0;
}
return flush_packet(s, pkt);
}
@@ -191,7 +194,7 @@ static int gif_write_trailer(AVFormatContext *s)
AVIOContext *pb = s->pb;
flush_packet(s, NULL);
- av_packet_unref(gif->prev_pkt);
+ av_packet_unref(&gif->prev_pkt);
avio_w8(pb, 0x3b);
return 0;
--
1.7.10.4
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
