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:

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index c3787e1..95838f7 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -391,7 +391,7 @@ int av_packet_ref(AVPacket *dst, AVPacket *src)
 {
     int ret;
 
-    ret = av_packet_copy_props(dst, src);
+    ret = av_copy_packet_side_data(dst, src);
     if (ret < 0)
         return ret;

and it works fine now. Not sure if the side data copying code in
av_packet_copy_props() can be replaced with the code in
av_copy_packet_side_data() without breaking anything though.

I've pushed this change to my fork now, if you want to try this.

> > > Output #0, gif, to 'test.ffmpeg.gif':
> > >     Stream #0:0: Video: gif, bgr8, 1920x1080 [SAR 1:1 DAR 16:9], q=2-31, 
> > > 200 kb/s, 100 tbn, 30 tbc
> > 
> > but when I use avconv, it selects "gray" (which is not what I want):
> > 
> > > Output #0, gif, to 'test.libav.gif':
> > >     Stream #0.0: Video: gif, gray, 1920x1080 [PAR 1:1 DAR 16:9], q=2-31, 
> > > 200 kb/s, 100 tbn, 1k tbc
> > 
> > and if I use "-pix_fmt bgr8" the output image is broken.
> > 
> > Compare [0] generated with ffmpeg, and [1] genenerated with "improved" 
> > avconv.
> > 
> > I've also tried copying lavu/pixdesc.{c,h} too, but nothing changed, and it
> > happens with all the other input files I've tried, so it shouldn't be 
> > specific
> > to the input file used.
> > 
> > Does anyone have an idea of what may be causing this? (e.g. if there's a 
> > special
> > avconv option I have to use, or if it is a bug in the gif encoder or in 
> > other
> > parts of libav).
> > 
> > If you want to test this I've setup a fork of libav.git at [2] with my WIP
> > patches (they still need to be split in proper commits) in the "gif" branch.
> > 
> 
> Seems vf_format behaves in a rather insane way, which results in this weird
> autoselection.

FWIW using ffmpeg's vf_format code doesn't change anything (it still picks
gray).

Cheers

Attachment: signature.asc
Description: Digital signature

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

Reply via email to