| Message ID | 20191002040412.821-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | c548b0a4c67b99e8728578a68084664d8bd220ac |
| Headers | show |
On Wed, Oct 02, 2019 at 06:04:10AM +0200, Andreas Rheinhardt wrote: > Up until now, aiffenc didn't rely on the standard functions for adding > an element to a linked list and freeing the list, but instead > reimplemented them. This has been changed. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavformat/aiffenc.c | 33 ++++----------------------------- > 1 file changed, 4 insertions(+), 29 deletions(-) > > diff --git a/libavformat/aiffenc.c b/libavformat/aiffenc.c > index aab37418f0..dd8b8c3d01 100644 > --- a/libavformat/aiffenc.c > +++ b/libavformat/aiffenc.c > @@ -36,7 +36,7 @@ typedef struct AIFFOutputContext { > int64_t frames; > int64_t ssnd; > int audio_stream_idx; > - AVPacketList *pict_list; > + AVPacketList *pict_list, *pict_list_end; > int write_id3v2; > int id3v2_version; > } AIFFOutputContext; > @@ -215,9 +215,6 @@ static int aiff_write_packet(AVFormatContext *s, AVPacket *pkt) > if (pkt->stream_index == aiff->audio_stream_idx) > avio_write(pb, pkt->data, pkt->size); > else { > - int ret; > - AVPacketList *pict_list, *last; > - > if (s->streams[pkt->stream_index]->codecpar->codec_type != AVMEDIA_TYPE_VIDEO) > return 0; > > @@ -229,24 +226,8 @@ static int aiff_write_packet(AVFormatContext *s, AVPacket *pkt) > if (s->streams[pkt->stream_index]->nb_frames >= 1) > return 0; > > - pict_list = av_mallocz(sizeof(AVPacketList)); > - if (!pict_list) > - return AVERROR(ENOMEM); > - > - ret = av_packet_ref(&pict_list->pkt, pkt); > - if (ret < 0) { > - av_freep(&pict_list); > - return ret; > - } > - > - if (!aiff->pict_list) > - aiff->pict_list = pict_list; > - else { > - last = aiff->pict_list; > - while (last->next) > - last = last->next; > - last->next = pict_list; > - } > + return ff_packet_list_put(&aiff->pict_list, &aiff->pict_list_end, > + pkt, FF_PACKETLIST_FLAG_REF_PACKET); > } > > return 0; > @@ -257,7 +238,6 @@ static int aiff_write_trailer(AVFormatContext *s) > int ret; > AVIOContext *pb = s->pb; > AIFFOutputContext *aiff = s->priv_data; > - AVPacketList *pict_list = aiff->pict_list; > AVCodecParameters *par = s->streams[aiff->audio_stream_idx]->codecpar; > > /* Chunks sizes must be even */ > @@ -293,12 +273,7 @@ static int aiff_write_trailer(AVFormatContext *s) > avio_flush(pb); > } > > - while (pict_list) { > - AVPacketList *next = pict_list->next; > - av_packet_unref(&pict_list->pkt); > - av_freep(&pict_list); > - pict_list = next; > - } > + ff_packet_list_free(&aiff->pict_list, &aiff->pict_list_end); > > return 0; > } > -- > 2.21.0 LGTM. Fate passes and patch tested with a few remux of aiff files containing id3v2 tags + attached pics (ffmpeg -i input.aiff -write_id3v2 1 output.aiff). Thanks.
On Wed, Oct 02, 2019 at 09:05:35AM +0200, Matthieu Bouron wrote: > On Wed, Oct 02, 2019 at 06:04:10AM +0200, Andreas Rheinhardt wrote: > > Up until now, aiffenc didn't rely on the standard functions for adding > > an element to a linked list and freeing the list, but instead > > reimplemented them. This has been changed. > > > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > > --- > > libavformat/aiffenc.c | 33 ++++----------------------------- > > 1 file changed, 4 insertions(+), 29 deletions(-) > > > > diff --git a/libavformat/aiffenc.c b/libavformat/aiffenc.c > > index aab37418f0..dd8b8c3d01 100644 > > --- a/libavformat/aiffenc.c > > +++ b/libavformat/aiffenc.c > > @@ -36,7 +36,7 @@ typedef struct AIFFOutputContext { > > int64_t frames; > > int64_t ssnd; > > int audio_stream_idx; > > - AVPacketList *pict_list; > > + AVPacketList *pict_list, *pict_list_end; > > int write_id3v2; > > int id3v2_version; > > } AIFFOutputContext; > > @@ -215,9 +215,6 @@ static int aiff_write_packet(AVFormatContext *s, AVPacket *pkt) > > if (pkt->stream_index == aiff->audio_stream_idx) > > avio_write(pb, pkt->data, pkt->size); > > else { > > - int ret; > > - AVPacketList *pict_list, *last; > > - > > if (s->streams[pkt->stream_index]->codecpar->codec_type != AVMEDIA_TYPE_VIDEO) > > return 0; > > > > @@ -229,24 +226,8 @@ static int aiff_write_packet(AVFormatContext *s, AVPacket *pkt) > > if (s->streams[pkt->stream_index]->nb_frames >= 1) > > return 0; > > > > - pict_list = av_mallocz(sizeof(AVPacketList)); > > - if (!pict_list) > > - return AVERROR(ENOMEM); > > - > > - ret = av_packet_ref(&pict_list->pkt, pkt); > > - if (ret < 0) { > > - av_freep(&pict_list); > > - return ret; > > - } > > - > > - if (!aiff->pict_list) > > - aiff->pict_list = pict_list; > > - else { > > - last = aiff->pict_list; > > - while (last->next) > > - last = last->next; > > - last->next = pict_list; > > - } > > + return ff_packet_list_put(&aiff->pict_list, &aiff->pict_list_end, > > + pkt, FF_PACKETLIST_FLAG_REF_PACKET); > > } > > > > return 0; > > @@ -257,7 +238,6 @@ static int aiff_write_trailer(AVFormatContext *s) > > int ret; > > AVIOContext *pb = s->pb; > > AIFFOutputContext *aiff = s->priv_data; > > - AVPacketList *pict_list = aiff->pict_list; > > AVCodecParameters *par = s->streams[aiff->audio_stream_idx]->codecpar; > > > > /* Chunks sizes must be even */ > > @@ -293,12 +273,7 @@ static int aiff_write_trailer(AVFormatContext *s) > > avio_flush(pb); > > } > > > > - while (pict_list) { > > - AVPacketList *next = pict_list->next; > > - av_packet_unref(&pict_list->pkt); > > - av_freep(&pict_list); > > - pict_list = next; > > - } > > + ff_packet_list_free(&aiff->pict_list, &aiff->pict_list_end); > > > > return 0; > > } > > -- > > 2.21.0 > > LGTM. > > Fate passes and patch tested with a few remux of aiff files containing > id3v2 tags + attached pics (ffmpeg -i input.aiff -write_id3v2 1 > output.aiff). will apply thx [...]
diff --git a/libavformat/aiffenc.c b/libavformat/aiffenc.c index aab37418f0..dd8b8c3d01 100644 --- a/libavformat/aiffenc.c +++ b/libavformat/aiffenc.c @@ -36,7 +36,7 @@ typedef struct AIFFOutputContext { int64_t frames; int64_t ssnd; int audio_stream_idx; - AVPacketList *pict_list; + AVPacketList *pict_list, *pict_list_end; int write_id3v2; int id3v2_version; } AIFFOutputContext; @@ -215,9 +215,6 @@ static int aiff_write_packet(AVFormatContext *s, AVPacket *pkt) if (pkt->stream_index == aiff->audio_stream_idx) avio_write(pb, pkt->data, pkt->size); else { - int ret; - AVPacketList *pict_list, *last; - if (s->streams[pkt->stream_index]->codecpar->codec_type != AVMEDIA_TYPE_VIDEO) return 0; @@ -229,24 +226,8 @@ static int aiff_write_packet(AVFormatContext *s, AVPacket *pkt) if (s->streams[pkt->stream_index]->nb_frames >= 1) return 0; - pict_list = av_mallocz(sizeof(AVPacketList)); - if (!pict_list) - return AVERROR(ENOMEM); - - ret = av_packet_ref(&pict_list->pkt, pkt); - if (ret < 0) { - av_freep(&pict_list); - return ret; - } - - if (!aiff->pict_list) - aiff->pict_list = pict_list; - else { - last = aiff->pict_list; - while (last->next) - last = last->next; - last->next = pict_list; - } + return ff_packet_list_put(&aiff->pict_list, &aiff->pict_list_end, + pkt, FF_PACKETLIST_FLAG_REF_PACKET); } return 0; @@ -257,7 +238,6 @@ static int aiff_write_trailer(AVFormatContext *s) int ret; AVIOContext *pb = s->pb; AIFFOutputContext *aiff = s->priv_data; - AVPacketList *pict_list = aiff->pict_list; AVCodecParameters *par = s->streams[aiff->audio_stream_idx]->codecpar; /* Chunks sizes must be even */ @@ -293,12 +273,7 @@ static int aiff_write_trailer(AVFormatContext *s) avio_flush(pb); } - while (pict_list) { - AVPacketList *next = pict_list->next; - av_packet_unref(&pict_list->pkt); - av_freep(&pict_list); - pict_list = next; - } + ff_packet_list_free(&aiff->pict_list, &aiff->pict_list_end); return 0; }
Up until now, aiffenc didn't rely on the standard functions for adding an element to a linked list and freeing the list, but instead reimplemented them. This has been changed. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavformat/aiffenc.c | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-)