[FFmpeg-devel] [PATCH 06/11] avcodec/encode, frame_thread_encoder: Unify calling encode callback

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

The encode-callback (the callback used by the FF_CODEC_CB_TYPE_ENCODE
encoders) is currently called in two places: encode_simple_internal()
and by the worker threads of frame-threaded encoders.

After the call, some packet properties are set based upon
the corresponding AVFrame properties and the packet is made
refcounted if it isn't already. So there is some code duplication.

There was also non-duplicated code in encode_simple_internal()
which is executed even when using frame-threading. This included
an emms_c() (which is needed for frame-threading, too, if it is
needed for the single-threaded case, because there are allocations
(via av_packet_make_refcounted()) immediately after returning
from the encode-callback).

Furthermore, some further properties are only set in
encode_simple_internal(): For audio, pts and duration are derived
from the corresponding fields of the frame if the encoder does not
have the AV_CODEC_CAP_DELAY set. Yet this is wrong for frame-threaded
encoders, because frame-threading always introduces delay regardless
of whether the underlying codec has said cap. This only worked because
there are no frame-threaded audio encoders.

This commit fixes the code duplication and the above issue by factoring
this code out and reusing it in both places. It would work in case
of audio codecs with frame-threading, because now the values are
derived from the correct AVFrame.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavcodec/encode.c               | 86 ++++++++++++++++---------------
 libavcodec/encode.h               |  3 ++
 libavcodec/frame_thread_encoder.c | 12 ++---
 3 files changed, 50 insertions(+), 51 deletions(-)

diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 01b59bbf70..f7b13c8ba1 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -172,6 +172,48 @@ int ff_encode_get_frame(AVCodecContext *avctx, AVFrame *frame)
     return 0;
 }
 
+int ff_encode_encode_cb(AVCodecContext *avctx, AVPacket *avpkt,
+                        const AVFrame *frame, int *got_packet)
+{
+    const FFCodec *const codec = ffcodec(avctx->codec);
+    int ret;
+
+    ret = codec->cb.encode(avctx, avpkt, frame, got_packet);
+    emms_c();
+    av_assert0(ret <= 0);
+
+    if (!ret && *got_packet) {
+        if (avpkt->data) {
+            ret = av_packet_make_refcounted(avpkt);
+            if (ret < 0)
+                goto unref;
+            // Date returned by encoders must always be ref-counted
+            av_assert0(avpkt->buf);
+        }
+
+        if (avctx->codec->type == AVMEDIA_TYPE_VIDEO &&
+            !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY))
+            avpkt->pts = avpkt->dts = frame->pts;
+        if (frame && !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY)) {
+            if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
+                if (avpkt->pts == AV_NOPTS_VALUE)
+                    avpkt->pts = frame->pts;
+                if (!avpkt->duration)
+                    avpkt->duration = ff_samples_to_time_base(avctx,
+                                                              frame->nb_samples);
+            }
+        }
+        if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
+            avpkt->dts = avpkt->pts;
+        }
+    } else {
+unref:
+        av_packet_unref(avpkt);
+    }
+
+    return ret;
+}
+
 static int encode_simple_internal(AVCodecContext *avctx, AVPacket *avpkt)
 {
     AVCodecInternal   *avci = avctx->internal;
@@ -204,58 +246,18 @@ static int encode_simple_internal(AVCodecContext *avctx, AVPacket *avpkt)
     av_assert0(codec->cb_type == FF_CODEC_CB_TYPE_ENCODE);
 
     if (CONFIG_FRAME_THREAD_ENCODER && avci->frame_thread_encoder)
-        /* This might modify frame, but it doesn't matter, because
-         * the frame properties used below are not used for video
-         * (due to the delay inherent in frame threaded encoding, it makes
-         *  no sense to use the properties of the current frame anyway). */
+        /* This might unref frame. */
         ret = ff_thread_video_encode_frame(avctx, avpkt, frame, &got_packet);
     else {
-        ret = codec->cb.encode(avctx, avpkt, frame, &got_packet);
-        if (avctx->codec->type == AVMEDIA_TYPE_VIDEO && !ret && got_packet &&
-            !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY))
-            avpkt->pts = avpkt->dts = frame->pts;
-    }
-
-    av_assert0(ret <= 0);
-
-    emms_c();
-
-    if (!ret && got_packet) {
-        if (avpkt->data) {
-            ret = av_packet_make_refcounted(avpkt);
-            if (ret < 0)
-                goto end;
-        }
-
-        if (frame && !(avctx->codec->capabilities & AV_CODEC_CAP_DELAY)) {
-            if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
-                if (avpkt->pts == AV_NOPTS_VALUE)
-                    avpkt->pts = frame->pts;
-                if (!avpkt->duration)
-                    avpkt->duration = ff_samples_to_time_base(avctx,
-                                                              frame->nb_samples);
-            }
-        }
-        if (avctx->codec->type == AVMEDIA_TYPE_AUDIO) {
-            avpkt->dts = avpkt->pts;
-        }
+        ret = ff_encode_encode_cb(avctx, avpkt, frame, &got_packet);
     }
 
     if (avci->draining && !got_packet)
         avci->draining_done = 1;
 
-end:
-    if (ret < 0 || !got_packet)
-        av_packet_unref(avpkt);
-
     if (frame)
         av_frame_unref(frame);
 
-    if (got_packet)
-        // Encoders must always return ref-counted buffers.
-        // Side-data only packets have no data and can be not ref-counted.
-        av_assert0(!avpkt->data || avpkt->buf);
-
     return ret;
 }
 
diff --git a/libavcodec/encode.h b/libavcodec/encode.h
index bc77918d8f..10c36435ad 100644
--- a/libavcodec/encode.h
+++ b/libavcodec/encode.h
@@ -75,4 +75,7 @@ int ff_alloc_packet(AVCodecContext *avctx, AVPacket *avpkt, int64_t size);
  */
 int ff_encode_preinit(AVCodecContext *avctx);
 
+int ff_encode_encode_cb(AVCodecContext *avctx, AVPacket *avpkt,
+                        const AVFrame *frame, int *got_packet);
+
 #endif /* AVCODEC_ENCODE_H */
diff --git a/libavcodec/frame_thread_encoder.c b/libavcodec/frame_thread_encoder.c
index b5765b6343..0d8134b258 100644
--- a/libavcodec/frame_thread_encoder.c
+++ b/libavcodec/frame_thread_encoder.c
@@ -29,6 +29,7 @@
 #include "libavutil/thread.h"
 #include "avcodec.h"
 #include "codec_internal.h"
+#include "encode.h"
 #include "internal.h"
 #include "pthread_internal.h"
 #include "thread.h"
@@ -80,7 +81,7 @@ static void * attribute_align_arg worker(void *v){
     ThreadContext *c = avctx->internal->frame_thread_encoder;
 
     while (!atomic_load(&c->exit)) {
-        int got_packet = 0, ret;
+        int ret;
         AVPacket *pkt;
         AVFrame *frame;
         Task *task;
@@ -105,14 +106,7 @@ static void * attribute_align_arg worker(void *v){
         frame = task->indata;
         pkt   = task->outdata;
 
-        ret = ffcodec(avctx->codec)->cb.encode(avctx, pkt, frame, &got_packet);
-        if(got_packet) {
-            int ret2 = av_packet_make_refcounted(pkt);
-            if (ret >= 0 && ret2 < 0)
-                ret = ret2;
-            pkt->pts = pkt->dts = frame->pts;
-        }
-        task->got_packet = got_packet;
+        ret = ff_encode_encode_cb(avctx, pkt, frame, &task->got_packet);
         pthread_mutex_lock(&c->buffer_mutex);
         av_frame_unref(frame);
         pthread_mutex_unlock(&c->buffer_mutex);
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".