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

Submitted by Andriy Gelman on April 21, 2019, 2:14 p.m.

Details

Message ID 20190421141414.3940-1-andriy.gelman@gmail.com
State New
Headers show

Commit Message

Andriy Gelman April 21, 2019, 2:14 p.m.
From: Andriy Gelman <andriy.gelman@gmail.com>

During AVPacket assignment, it is currently not clear when the lhs takes
ownership of the packet. This commit replaces assignment with an
explicit av_packet_move_ref call when there is an ownership transfer to
clear the distinction.
---
 libavformat/utils.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

Comments

Andriy Gelman May 19, 2019, 7:18 p.m.
On Sun, 21. Apr 10:14, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> During AVPacket assignment, it is currently not clear when the lhs takes
> ownership of the packet. This commit replaces assignment with an
> explicit av_packet_move_ref call when there is an ownership transfer to
> clear the distinction.
> ---
>  libavformat/utils.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 9b3f0d28e6..9653ebb4f4 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;
> +                s->internal->raw_packet_buffer_remaining_size += buffer_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))
> -- 
> 2.21.0
> 

ping pls

Thanks, 
Andriy

Patch hide | download patch | download mbox

diff --git a/libavformat/utils.c b/libavformat/utils.c
index 9b3f0d28e6..9653ebb4f4 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;
+                s->internal->raw_packet_buffer_remaining_size += buffer_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))