diff mbox

[FFmpeg-devel] avformat/utils: Use av_packet_move_ref for packet ownership transfer

Message ID 20190404145503.20761-1-andriy.gelman@gmail.com
State Superseded
Headers show

Commit Message

Andriy Gelman April 4, 2019, 2:55 p.m. UTC 
From: Andriy Gelman <andriy.gelman@gmail.com>

This commit replaces packet assignment operator with av_packet_move_ref when there
is a packet ownership transfer.
---

Aims to address a TODO in libavformat/utils.c about using av_packet_mov_ref
instead of assignment operator.

 libavformat/utils.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

Comments

Michael Niedermayer April 4, 2019, 6:29 p.m. UTC | #1 
On Thu, Apr 04, 2019 at 10:55:03AM -0400, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> This commit replaces packet assignment operator with av_packet_move_ref when there
> is a packet ownership transfer.
> ---
> 
> Aims to address a TODO in libavformat/utils.c about using av_packet_mov_ref
> instead of assignment operator.
> 
>  libavformat/utils.c | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)

this changes the behavior:
./ffprobe ~/tickets/4221/teletext-streams-misrecognized.ts

https://samples.ffmpeg.org/ffmpeg-bugs/trac/ticket4221/teletext-streams-misrecognized.ts

before:
    Stream #0:0[0x65]: Video: mpeg2video (Main), yuv420p(tv, top first), 720x576 [SAR 64:45 DAR 16:9], 25 fps, 25 tbr, 90k tbn, 50 tbc
    Stream #0:1[0x68]: Unknown: none
    Stream #0:2[0x6a]: Audio: ac3, 48000 Hz, stereo, fltp, 448 kb/s
    Stream #0:3[0x67]: Audio: mp2, 48000 Hz, stereo, s16p, 192 kb/s
    Stream #0:4[0x66]: Audio: mp2, 48000 Hz, stereo, s16p, 256 kb/s
    Stream #0:5[0x69]: Subtitle: dvb_subtitle
    
afterwards:
    Stream #0:0[0x65]: Video: mpeg2video (Main), yuv420p(tv, top first), 720x576 [SAR 64:45 DAR 16:9], 25 fps, 25 tbr, 90k tbn, 50 tbc
    Stream #0:1[0x68]: Unknown: none
    Stream #0:2[0x6a]: Audio: ac3, 48000 Hz, stereo, fltp, 448 kb/s
    Stream #0:3[0x67]: Audio: mp2, 48000 Hz, stereo, s16p, 192 kb/s
    Stream #0:4[0x66]: Audio: mp2, 48000 Hz, stereo, s16p, 256 kb/s
    Stream #0:5[0x69]: Unknown: none


[...]
diff mbox

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 9b3f0d28e6..f757b02d49 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -460,10 +460,7 @@  int ff_packet_list_put(AVPacketList **packet_buffer,
             return ret;
         }
     } else {
-        // TODO: Adapt callers in this file so the line below can use
-        //       av_packet_move_ref() to effectively move the reference
-        //       to the list.
-        pktl->pkt = *pkt;
+        av_packet_move_ref(&pktl->pkt, pkt);
     }
 
     if (*packet_buffer)
@@ -832,19 +829,21 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
 {
     int ret, i, err;
     AVStream *st;
+    AVPacket *buffer_pkt; /*view packet contents only. does not take ownership*/
 
     for (;;) {
         AVPacketList *pktl = s->internal->raw_packet_buffer;
 
         if (pktl) {
-            *pkt = pktl->pkt;
-            st   = s->streams[pkt->stream_index];
+            buffer_pkt = &pktl->pkt;
+            st   = s->streams[buffer_pkt->stream_index];
             if (s->internal->raw_packet_buffer_remaining_size <= 0)
                 if ((err = probe_codec(s, st, NULL)) < 0)
                     return err;
             if (st->request_probe <= 0) {
                 s->internal->raw_packet_buffer                 = pktl->next;
                 s->internal->raw_packet_buffer_remaining_size += pkt->size;
+                av_packet_move_ref(pkt, &pktl->pkt);
                 av_free(pktl);
                 return 0;
             }
@@ -914,14 +913,17 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
         if (!pktl && st->request_probe <= 0)
             return ret;
 
+        /*transfer pkt ownership to list*/
         err = ff_packet_list_put(&s->internal->raw_packet_buffer,
                                  &s->internal->raw_packet_buffer_end,
                                  pkt, 0);
         if (err)
             return err;
-        s->internal->raw_packet_buffer_remaining_size -= pkt->size;
 
-        if ((err = probe_codec(s, st, pkt)) < 0)
+        buffer_pkt = &s->internal->raw_packet_buffer_end->pkt;
+        s->internal->raw_packet_buffer_remaining_size -= buffer_pkt->size;
+
+        if ((err = probe_codec(s, st, buffer_pkt)) < 0)
             return err;
     }
 }
@@ -1662,7 +1664,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
 
         if (!st->need_parsing || !st->parser) {
             /* no parsing needed: we just output the packet as is */
-            *pkt = cur_pkt;
+            av_packet_move_ref(pkt, &cur_pkt);
             compute_pkt_fields(s, st, NULL, pkt, AV_NOPTS_VALUE, AV_NOPTS_VALUE);
             if ((s->iformat->flags & AVFMT_GENERIC_INDEX) &&
                 (pkt->flags & AV_PKT_FLAG_KEY) && pkt->dts != AV_NOPTS_VALUE) {
@@ -3779,15 +3781,15 @@  FF_ENABLE_DEPRECATION_WARNINGS
             break;
         }
 
-        pkt = &pkt1;
-
         if (!(ic->flags & AVFMT_FLAG_NOBUFFER)) {
             ret = ff_packet_list_put(&ic->internal->packet_buffer,
                                      &ic->internal->packet_buffer_end,
-                                     pkt, 0);
+                                     &pkt1, 0);
             if (ret < 0)
                 goto find_stream_info_err;
-        }
+            pkt = &ic->internal->packet_buffer_end->pkt;
+        } else
+            pkt = &pkt1;
 
         st = ic->streams[pkt->stream_index];
         if (!(st->disposition & AV_DISPOSITION_ATTACHED_PIC))