On 2 August 2017 at 15:30, James Almer <jamr...@gmail.com> wrote: > On 8/2/2017 3:30 AM, Rodger Combs wrote: > > --- > > libavformat/flacenc.c | 285 ++++++++++++++++++++++++++++++ > +++++++++++++------- > > 1 file changed, 250 insertions(+), 35 deletions(-) > > > > diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c > > index b894f9e..9768b6a 100644 > > [...] > > > @@ -166,23 +341,63 @@ static int flac_write_trailer(struct > AVFormatContext *s) > > static int flac_write_packet(struct AVFormatContext *s, AVPacket *pkt) > > { > > FlacMuxerContext *c = s->priv_data; > > - uint8_t *streaminfo; > > - int streaminfo_size; > > + if (pkt->stream_index == c->audio_stream_idx) { > > + if (c->waiting_pics) { > > + /* buffer audio packets until we get all the pictures */ > > + AVPacketList *pktl = av_mallocz(sizeof(*pktl)); > > + int ret; > > + if (!pktl) { > > + ret = AVERROR(ENOMEM); > > +oom: > > + if (s->error_recognition & AV_EF_EXPLODE) { > > + av_free(pktl); > > + return ret; > > + } > > + av_log(s, AV_LOG_ERROR, "Out of memory in packet queue; > skipping attached pictures\n"); > > + c->waiting_pics = 0; > > + if ((ret = flac_queue_flush(s)) < 0) > > + return ret; > > + return flac_write_audio_packet(s, pkt); > > + } > > + > > + ret = av_packet_ref(&pktl->pkt, pkt); > > + if (ret < 0) { > > + av_freep(&pktl); > > + goto oom; > > + } > > + > > + if (c->queue_end) > > + c->queue_end->next = pktl; > > + else > > + c->queue = pktl; > > + c->queue_end = pktl; > > + } else { > > + return flac_write_audio_packet(s, pkt); > > + } > > + } else { > > + int ret, index = pkt->stream_index; > > > > - /* check for updated streaminfo */ > > - streaminfo = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA, > > - &streaminfo_size); > > - if (streaminfo && streaminfo_size == FLAC_STREAMINFO_SIZE) { > > - av_freep(&c->streaminfo); > > + /* warn only once for each stream */ > > + if (s->streams[pkt->stream_index]->nb_frames == 1) { > > + av_log(s, AV_LOG_WARNING, "Got more than one picture in > stream %d," > > + " ignoring.\n", pkt->stream_index); > > + } > > + if (!c->waiting_pics || s->streams[pkt->stream_index]->nb_frames > >= 1) > > + return 0; > > > > - c->streaminfo = av_malloc(FLAC_STREAMINFO_SIZE); > > - if (!c->streaminfo) > > - return AVERROR(ENOMEM); > > - memcpy(c->streaminfo, streaminfo, FLAC_STREAMINFO_SIZE); > > + if (index > c->audio_stream_idx) > > + index--; > > + > > + if ((ret = av_copy_packet(&c->pics[index], pkt)) < 0) > > Shouldn't this be av_packet_ref()? > And they should probably be unreferenced after being consumed in > flac_finish_header(), much like the audio packets are in > flac_queue_flush(). > > Also, you don't seem to be freeing c->pics anywhere. > > av_packet_copy does a ref if the data is refcounted so its okay. c->pics is indeed not freed, as well as the pictures it refs
If you free the list of pics and the pics themselves during flac_finish_header() the patch will LGTM _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel