diff mbox series

[FFmpeg-devel,14/18] avformat/flacenc: port to the new packet list API

Message ID 20201118165247.4130-15-jamrial@gmail.com
State Superseded
Headers show
Series AVPacketList public API | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished

Commit Message

James Almer Nov. 18, 2020, 4:52 p.m. UTC 
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/flacenc.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Michael Niedermayer Nov. 19, 2020, 4:42 p.m. UTC | #1 
On Wed, Nov 18, 2020 at 01:52:43PM -0300, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/flacenc.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

crashes
-i ~/bug/857/rythmortis.flac -t 5 -bitexact -y 24bit.flac
not sure where the sample is, i couldnt find it on the server

==22032== Conditional jump or move depends on uninitialised value(s)
==22032==    at 0x585483: flac_write_audio_packet (flacenc.c:297)
==22032==    by 0x585526: flac_queue_flush (flacenc.c:313)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==    by 0x63F2D9: write_packet (mux.c:731)
==22032==    by 0x64063B: interleaved_write_packet (mux.c:1120)
==22032==    by 0x6407FA: write_packet_common (mux.c:1145)
==22032==    by 0x640AC0: write_packets_common (mux.c:1202)
==22032==    by 0x640C7F: av_interleaved_write_frame (mux.c:1257)
==22032==    by 0x24B955: write_packet (ffmpeg.c:837)
==22032==    by 0x24BB4F: output_packet (ffmpeg.c:883)
==22032==    by 0x250A8B: flush_encoders (ffmpeg.c:1995)
==22032==    by 0x25B73D: transcode (ffmpeg.c:4779)
==22032==    by 0x25BF7E: main (ffmpeg.c:4964)
==22032==  Uninitialised value was created by a stack allocation
==22032==    at 0x5854C2: flac_queue_flush (flacenc.c:303)
==22032== 
==22032== Conditional jump or move depends on uninitialised value(s)
==22032==    at 0x54F732: avio_write (aviobuf.c:232)
==22032==    by 0x5854A6: flac_write_audio_packet (flacenc.c:298)
==22032==    by 0x585526: flac_queue_flush (flacenc.c:313)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==    by 0x63F2D9: write_packet (mux.c:731)
==22032==    by 0x64063B: interleaved_write_packet (mux.c:1120)
==22032==    by 0x6407FA: write_packet_common (mux.c:1145)
==22032==    by 0x640AC0: write_packets_common (mux.c:1202)
==22032==    by 0x640C7F: av_interleaved_write_frame (mux.c:1257)
==22032==    by 0x24B955: write_packet (ffmpeg.c:837)
==22032==    by 0x24BB4F: output_packet (ffmpeg.c:883)
==22032==    by 0x250A8B: flush_encoders (ffmpeg.c:1995)
==22032==    by 0x25B73D: transcode (ffmpeg.c:4779)
==22032==    by 0x25BF7E: main (ffmpeg.c:4964)
==22032==  Uninitialised value was created by a stack allocation
==22032==    at 0x5854C2: flac_queue_flush (flacenc.c:303)
==22032== 
==22032== Conditional jump or move depends on uninitialised value(s)
==22032==    at 0x76FC7F: av_packet_free_side_data (avpacket.c:276)
==22032==    by 0x770BB0: av_packet_unref (avpacket.c:608)
==22032==    by 0x585542: flac_queue_flush (flacenc.c:315)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==    by 0x63F2D9: write_packet (mux.c:731)
==22032==    by 0x64063B: interleaved_write_packet (mux.c:1120)
==22032==    by 0x6407FA: write_packet_common (mux.c:1145)
==22032==    by 0x640AC0: write_packets_common (mux.c:1202)
==22032==    by 0x640C7F: av_interleaved_write_frame (mux.c:1257)
==22032==    by 0x24B955: write_packet (ffmpeg.c:837)
==22032==    by 0x24BB4F: output_packet (ffmpeg.c:883)
==22032==    by 0x250A8B: flush_encoders (ffmpeg.c:1995)
==22032==    by 0x25B73D: transcode (ffmpeg.c:4779)
==22032==    by 0x25BF7E: main (ffmpeg.c:4964)
==22032==  Uninitialised value was created by a stack allocation
==22032==    at 0x5854C2: flac_queue_flush (flacenc.c:303)
==22032== 
==22032== Use of uninitialised value of size 8
==22032==    at 0x120A88B: av_freep (mem.c:232)
==22032==    by 0x76FC70: av_packet_free_side_data (avpacket.c:277)
==22032==    by 0x770BB0: av_packet_unref (avpacket.c:608)
==22032==    by 0x585542: flac_queue_flush (flacenc.c:315)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==    by 0x63F2D9: write_packet (mux.c:731)
==22032==    by 0x64063B: interleaved_write_packet (mux.c:1120)
==22032==    by 0x6407FA: write_packet_common (mux.c:1145)
==22032==    by 0x640AC0: write_packets_common (mux.c:1202)
==22032==    by 0x640C7F: av_interleaved_write_frame (mux.c:1257)
==22032==    by 0x24B955: write_packet (ffmpeg.c:837)
==22032==    by 0x24BB4F: output_packet (ffmpeg.c:883)
==22032==    by 0x250A8B: flush_encoders (ffmpeg.c:1995)
==22032==    by 0x25B73D: transcode (ffmpeg.c:4779)
==22032==    by 0x25BF7E: main (ffmpeg.c:4964)
==22032==  Uninitialised value was created by a stack allocation
==22032==    at 0x5854C2: flac_queue_flush (flacenc.c:303)
==22032== 
==22032== Use of uninitialised value of size 8
==22032==    at 0x120A8A2: av_freep (mem.c:233)
==22032==    by 0x76FC70: av_packet_free_side_data (avpacket.c:277)
==22032==    by 0x770BB0: av_packet_unref (avpacket.c:608)
==22032==    by 0x585542: flac_queue_flush (flacenc.c:315)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==    by 0x63F2D9: write_packet (mux.c:731)
==22032==    by 0x64063B: interleaved_write_packet (mux.c:1120)
==22032==    by 0x6407FA: write_packet_common (mux.c:1145)
==22032==    by 0x640AC0: write_packets_common (mux.c:1202)
==22032==    by 0x640C7F: av_interleaved_write_frame (mux.c:1257)
==22032==    by 0x24B955: write_packet (ffmpeg.c:837)
==22032==    by 0x24BB4F: output_packet (ffmpeg.c:883)
==22032==    by 0x250A8B: flush_encoders (ffmpeg.c:1995)
==22032==    by 0x25B73D: transcode (ffmpeg.c:4779)
==22032==    by 0x25BF7E: main (ffmpeg.c:4964)
==22032==  Uninitialised value was created by a stack allocation
==22032==    at 0x5854C2: flac_queue_flush (flacenc.c:303)
==22032== 
==22032== Invalid free() / delete / delete[] / realloc()
==22032==    at 0x4C32D3B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22032==    by 0x120A868: av_free (mem.c:224)
==22032==    by 0x120A8B0: av_freep (mem.c:234)
==22032==    by 0x76FC70: av_packet_free_side_data (avpacket.c:277)
==22032==    by 0x770BB0: av_packet_unref (avpacket.c:608)
==22032==    by 0x585542: flac_queue_flush (flacenc.c:315)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==    by 0x1FFEFFFAEF: ???
==22032==    by 0x16920EFF: ???
==22032==    by 0x1FFEFFFAEF: ???
==22032==    by 0x16920EFF: ???
==22032==    by 0x16921A7F: ???
==22032==    by 0x16921B7F: ???
==22032==    by 0x1FFEFFFAAF: ???
==22032==    by 0x63F2D9: write_packet (mux.c:731)
==22032==    by 0x1FFEFFFAEF: ???
==22032==    by 0x16920EFF: ???
==22032==  Address 0x1ffefff9f0 is on thread 1's stack
==22032== 
==22032== Conditional jump or move depends on uninitialised value(s)
==22032==    at 0x4C32CF1: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==22032==    by 0x120A868: av_free (mem.c:224)
==22032==    by 0x120A8B0: av_freep (mem.c:234)
==22032==    by 0x76FC70: av_packet_free_side_data (avpacket.c:277)
==22032==    by 0x770BB0: av_packet_unref (avpacket.c:608)
==22032==    by 0x585542: flac_queue_flush (flacenc.c:315)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==  Uninitialised value was created by a stack allocation
==22032==    at 0x5856B0: flac_write_packet (flacenc.c:356)
==22032== 
==22032== Invalid read of size 8
==22032==    at 0x120A88B: av_freep (mem.c:232)
==22032==    by 0x76FC70: av_packet_free_side_data (avpacket.c:277)
==22032==    by 0x770BB0: av_packet_unref (avpacket.c:608)
==22032==    by 0x585542: flac_queue_flush (flacenc.c:315)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==  Address 0x1fff001000 is not stack'd, malloc'd or (recently) free'd
==22032== 
==22032== 
==22032== Process terminating with default action of signal 11 (SIGSEGV)
==22032==  Access not within mapped region at address 0x1FFF001000
==22032==    at 0x120A88B: av_freep (mem.c:232)
==22032==    by 0x76FC70: av_packet_free_side_data (avpacket.c:277)
==22032==    by 0x770BB0: av_packet_unref (avpacket.c:608)
==22032==    by 0x585542: flac_queue_flush (flacenc.c:315)
==22032==    by 0x585887: flac_write_packet (flacenc.c:395)
==22032==  If you believe this happened as a result of a stack
==22032==  overflow in your program's main thread (unlikely but
==22032==  possible), you can try to increase the size of the
==22032==  main thread stack using the --main-stacksize= flag.
==22032==  The main thread stack size used in this run was 8388608.

[...]
James Almer Nov. 19, 2020, 4:49 p.m. UTC | #2 
On 11/19/2020 1:42 PM, Michael Niedermayer wrote:
> On Wed, Nov 18, 2020 at 01:52:43PM -0300, James Almer wrote:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavformat/flacenc.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> crashes
> -i ~/bug/857/rythmortis.flac -t 5 -bitexact -y 24bit.flac
> not sure where the sample is, i couldnt find it on the server

I think i found the problem. Guess there's no FATE test muxing flac with 
cover art...
diff mbox series

Patch

diff --git a/libavformat/flacenc.c b/libavformat/flacenc.c
index a24d3be85d..6fb5e59be0 100644
--- a/libavformat/flacenc.c
+++ b/libavformat/flacenc.c
@@ -39,7 +39,7 @@  typedef struct FlacMuxerContext {
     int audio_stream_idx;
     int waiting_pics;
     /* audio packets are queued here until we get all the attached pictures */
-    PacketListEntry *queue, *queue_end;
+    AVPacketList *queue;
 
     /* updated streaminfo sent by the encoder at the end */
     uint8_t streaminfo[FLAC_STREAMINFO_SIZE];
@@ -253,6 +253,10 @@  static int flac_init(struct AVFormatContext *s)
         }
     }
 
+    c->queue = av_packet_list_alloc();
+    if (!c->queue)
+        return AVERROR(ENOMEM);
+
     return 0;
 }
 
@@ -305,8 +309,7 @@  static int flac_queue_flush(AVFormatContext *s)
     if (ret < 0)
         write = 0;
 
-    while (c->queue) {
-        avpriv_packet_list_get(&c->queue, &c->queue_end, &pkt);
+    while (av_packet_list_get(c->queue, &pkt, 0)) {
         if (write && (ret = flac_write_audio_packet(s, &pkt)) < 0)
             write = 0;
         av_packet_unref(&pkt);
@@ -346,7 +349,7 @@  static void flac_deinit(struct AVFormatContext *s)
 {
     FlacMuxerContext *c = s->priv_data;
 
-    avpriv_packet_list_free(&c->queue, &c->queue_end);
+    av_packet_list_free(&c->queue);
 }
 
 static int flac_write_packet(struct AVFormatContext *s, AVPacket *pkt)
@@ -357,7 +360,7 @@  static int flac_write_packet(struct AVFormatContext *s, AVPacket *pkt)
     if (pkt->stream_index == c->audio_stream_idx) {
         if (c->waiting_pics) {
             /* buffer audio packets until we get all the pictures */
-            ret = avpriv_packet_list_put(&c->queue, &c->queue_end, pkt, av_packet_ref, 0);
+            ret = av_packet_list_put(c->queue, pkt, av_packet_ref, 0);
             if (ret < 0) {
                 av_log(s, AV_LOG_ERROR, "Out of memory in packet queue; skipping attached pictures\n");
                 c->waiting_pics = 0;