On mer, mag 14, 2014 at 04:27:55 +0200, Luca Barbato wrote:
> On 14/05/14 15:56, Alessandro Ghedini 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);
> 
> What looks strange is that seems to keeping a packet around and writing
> over it, why it is doing that?
> 
> Since we keep a _reference_ it overwrites not a copy but the same data.

What do you mean by "writing over it"? AFAICT the new packet data is "copied"
to prev_pkt only after prev_pkt's data has already been avio_written:

> >      avio_write(pb, pkt->data, pkt->size);
> >  
> >      av_packet_unref(gif->prev_pkt);
> >      if (new)
> >          av_copy_packet(gif->prev_pkt, new);

Here "pkt" and "gif->prev_pkt" points to the same packet.

Attachment: signature.asc
Description: Digital signature

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

Reply via email to