diff mbox series

[FFmpeg-devel,03/13] avcodec/decode: support applying LCEVC enhacements

Message ID 20240831163114.4197-3-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,01/13,v3] avutil/frame: add an LCEVC enhancement data payload side data type | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

James Almer Aug. 31, 2024, 4:31 p.m. UTC
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/avcodec.c       |  2 ++
 libavcodec/avcodec.h       |  5 +++++
 libavcodec/decode.c        | 42 +++++++++++++++++++++++++++++++++++++-
 libavcodec/internal.h      |  2 ++
 libavcodec/lcevcdec.c      |  2 ++
 libavcodec/options_table.h |  1 +
 libavcodec/pthread_frame.c |  7 +++++++
 7 files changed, 60 insertions(+), 1 deletion(-)

Comments

Derek Buitenhuis Sept. 2, 2024, 12:03 p.m. UTC | #1
On 8/31/2024 5:31 PM, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/avcodec.c       |  2 ++
>  libavcodec/avcodec.h       |  5 +++++
>  libavcodec/decode.c        | 42 +++++++++++++++++++++++++++++++++++++-
>  libavcodec/internal.h      |  2 ++
>  libavcodec/lcevcdec.c      |  2 ++
>  libavcodec/options_table.h |  1 +
>  libavcodec/pthread_frame.c |  7 +++++++
>  7 files changed, 60 insertions(+), 1 deletion(-)

No real opinion on the other patches, but this one feels really gross. I feel
liek we should not be adding new codec-specific things in AVCodecInternal or
in generic threding, decode, etc. It really feels quite wrong.

I probably missed discussion on this in previous revisions of this patch set,
but I wanted to get my thoughts in writing.

- Derek
James Almer Sept. 2, 2024, 4:48 p.m. UTC | #2
On 9/2/2024 9:03 AM, Derek Buitenhuis wrote:
> On 8/31/2024 5:31 PM, James Almer wrote:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/avcodec.c       |  2 ++
>>   libavcodec/avcodec.h       |  5 +++++
>>   libavcodec/decode.c        | 42 +++++++++++++++++++++++++++++++++++++-
>>   libavcodec/internal.h      |  2 ++
>>   libavcodec/lcevcdec.c      |  2 ++
>>   libavcodec/options_table.h |  1 +
>>   libavcodec/pthread_frame.c |  7 +++++++
>>   7 files changed, 60 insertions(+), 1 deletion(-)
> 
> No real opinion on the other patches, but this one feels really gross. I feel
> liek we should not be adding new codec-specific things in AVCodecInternal or
> in generic threding, decode, etc. It really feels quite wrong.

I could try to make it a bit more opaque to decode.c, making i call a 
function that would then set any needed post processing callback or 
something like that.

> 
> I probably missed discussion on this in previous revisions of this patch set,
> but I wanted to get my thoughts in writing.
Jean-Baptiste Kempf Sept. 2, 2024, 7:57 p.m. UTC | #3
On Mon, 2 Sep 2024, at 18:48, James Almer wrote:
> On 9/2/2024 9:03 AM, Derek Buitenhuis wrote:
>> On 8/31/2024 5:31 PM, James Almer wrote:
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>   libavcodec/avcodec.c       |  2 ++
>>>   libavcodec/avcodec.h       |  5 +++++
>>>   libavcodec/decode.c        | 42 +++++++++++++++++++++++++++++++++++++-
>>>   libavcodec/internal.h      |  2 ++
>>>   libavcodec/lcevcdec.c      |  2 ++
>>>   libavcodec/options_table.h |  1 +
>>>   libavcodec/pthread_frame.c |  7 +++++++
>>>   7 files changed, 60 insertions(+), 1 deletion(-)
>> 
>> No real opinion on the other patches, but this one feels really gross. I feel
>> liek we should not be adding new codec-specific things in AVCodecInternal or
>> in generic threding, decode, etc. It really feels quite wrong.
>
> I could try to make it a bit more opaque to decode.c, making i call a 
> function that would then set any needed post processing callback or 
> something like that.

If you could, that would be neat: I was also frustrated by this patch in the serie.
Ronald S. Bultje Sept. 2, 2024, 8:10 p.m. UTC | #4
Hi,

On Sat, Aug 31, 2024 at 12:31 PM James Almer <jamrial@gmail.com> wrote:

> --- a/libavcodec/options_table.h
> +++ b/libavcodec/options_table.h
> @@ -93,6 +93,7 @@ static const AVOption avcodec_options[] = {
>
[..]

> {"film_grain", "export film grain parameters through frame side data", 0,
> AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_FILM_GRAIN}, INT_MIN,
> INT_MAX, V|D, .unit = "export_side_data"},
> +{"enhancements", "export film grain parameters through frame side data",
> 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_ENHANCEMENTS}, INT_MIN,
> INT_MAX, V|D, .unit = "export_side_data"},
>

Copy/paste typo? Maybe change to "export stream enhancements through frame
side data".

Ronald
diff mbox series

Patch

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 6065f1b689..380df267a0 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -446,6 +446,8 @@  av_cold void ff_codec_close(AVCodecContext *avctx)
         ff_refstruct_unref(&avci->pool);
         ff_refstruct_pool_uninit(&avci->progress_frame_pool);
 
+        av_buffer_unref(&avci->lcevc);
+
         ff_hwaccel_uninit(avctx);
 
         av_bsf_free(&avci->bsf);
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7a67300134..f6e01da738 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -418,6 +418,11 @@  typedef struct RcOverride{
  * Do not apply film grain, export it instead.
  */
 #define AV_CODEC_EXPORT_DATA_FILM_GRAIN (1 << 3)
+/**
+ * Decoding only.
+ * Do not apply picture enhancement layers, export them instead.
+ */
+#define AV_CODEC_EXPORT_DATA_ENHANCEMENTS (1 << 4)
 
 /**
  * The decoder will keep a reference to the frame and may reuse it later.
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 27dba8a1f3..dc868714ac 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -48,6 +48,7 @@ 
 #include "hwaccel_internal.h"
 #include "hwconfig.h"
 #include "internal.h"
+#include "lcevcdec.h"
 #include "packet_internal.h"
 #include "progressframe.h"
 #include "refstruct.h"
@@ -1599,6 +1600,7 @@  int ff_attach_decode_data(AVFrame *frame)
 int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
 {
     const FFHWAccel *hwaccel = ffhwaccel(avctx->hwaccel);
+    int lcevc = 0, width = 0, height = 0;
     int override_dimensions = 1;
     int ret;
 
@@ -1639,8 +1641,17 @@  int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
             ret = hwaccel->alloc_frame(avctx, frame);
             goto end;
         }
-    } else
+    } else {
         avctx->sw_pix_fmt = avctx->pix_fmt;
+        lcevc = CONFIG_LIBLCEVC_DEC && avctx->codec_type == AVMEDIA_TYPE_VIDEO &&
+                avctx->internal->lcevc && av_frame_get_side_data(frame, AV_FRAME_DATA_LCEVC);
+        if (lcevc) {
+            width         = frame->width;
+            height        = frame->height;
+            frame->width  = frame->width  * 2 / FFMAX(frame->sample_aspect_ratio.den, 1);
+            frame->height = frame->height * 2 / FFMAX(frame->sample_aspect_ratio.num, 1);
+        }
+    }
 
     ret = avctx->get_buffer2(avctx, frame, flags);
     if (ret < 0)
@@ -1652,6 +1663,20 @@  int ff_get_buffer(AVCodecContext *avctx, AVFrame *frame, int flags)
     if (ret < 0)
         goto fail;
 
+    if (CONFIG_LIBLCEVC_DEC && lcevc) {
+        FrameDecodeData *fdd = (FrameDecodeData*)frame->private_ref->data;
+        fdd->post_process_opaque = av_buffer_ref(avctx->internal->lcevc);
+
+        frame->width  = width;
+        frame->height = height;
+        if (!fdd->post_process_opaque) {
+            ret = AVERROR(ENOMEM);
+            goto fail;
+        }
+        fdd->post_process_opaque_free = ff_lcevc_unref;
+        fdd->post_process = ff_lcevc_process;
+    }
+
 end:
     if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions &&
         !(ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_EXPORTS_CROPPING)) {
@@ -1949,6 +1974,21 @@  int ff_decode_preinit(AVCodecContext *avctx)
     if (ret < 0)
         return ret;
 
+    if (CONFIG_LIBLCEVC_DEC && !(avctx->export_side_data & AV_CODEC_EXPORT_DATA_ENHANCEMENTS)) {
+        FFLCEVCContext *lcevc = av_mallocz(sizeof(*lcevc));
+
+        if (!lcevc || !(avci->lcevc = av_buffer_create((uint8_t *)lcevc, sizeof(*lcevc),
+                                                       ff_lcevc_free, lcevc, 0))) {
+            int explode = avctx->err_recognition & AV_EF_EXPLODE;
+            av_log(avctx, explode ? AV_LOG_ERROR: AV_LOG_WARNING,
+                   "Error allocating LCEVC context\n");
+            if (explode) {
+                av_free(lcevc);
+                return AVERROR(ENOMEM);
+            }
+        }
+    }
+
 #if FF_API_DROPCHANGED
     if (avctx->flags & AV_CODEC_FLAG_DROPCHANGED)
         av_log(avctx, AV_LOG_WARNING, "The dropchanged flag is deprecated.\n");
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 98ab2797ce..aae78854cc 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -158,6 +158,8 @@  typedef struct AVCodecInternal {
     FFIccContext icc; /* used to read and write embedded ICC profiles */
 #endif
 
+    AVBufferRef *lcevc;
+
     /**
      * Set when the user has been warned about a failed allocation from
      * a fixed frame pool.
diff --git a/libavcodec/lcevcdec.c b/libavcodec/lcevcdec.c
index ae1d150986..47fdaaa9ef 100644
--- a/libavcodec/lcevcdec.c
+++ b/libavcodec/lcevcdec.c
@@ -229,6 +229,8 @@  int ff_lcevc_process(void *logctx, AVFrame *frame)
     if (ret < 0)
         return ret;
 
+    av_frame_remove_side_data(frame, AV_FRAME_DATA_LCEVC);
+
     return 0;
 }
 
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index 33f1bce887..332f354fa6 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -93,6 +93,7 @@  static const AVOption avcodec_options[] = {
 {"prft", "export Producer Reference Time through packet side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_PRFT}, INT_MIN, INT_MAX, A|V|S|E, .unit = "export_side_data"},
 {"venc_params", "export video encoding parameters through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, INT_MIN, INT_MAX, V|D, .unit = "export_side_data"},
 {"film_grain", "export film grain parameters through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_FILM_GRAIN}, INT_MIN, INT_MAX, V|D, .unit = "export_side_data"},
+{"enhancements", "export film grain parameters through frame side data", 0, AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_ENHANCEMENTS}, INT_MIN, INT_MAX, V|D, .unit = "export_side_data"},
 {"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0}, 0, INT_MAX},
 {"g", "set the group of picture (GOP) size", OFFSET(gop_size), AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
 {"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 019e33b7b2..00ad93cbdf 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -782,6 +782,7 @@  void ff_frame_thread_free(AVCodecContext *avctx, int thread_count)
             ff_refstruct_unref(&ctx->internal->pool);
             av_packet_free(&ctx->internal->in_pkt);
             av_packet_free(&ctx->internal->last_pkt_props);
+            av_buffer_unref(&ctx->internal->lcevc);
             av_freep(&ctx->internal);
             av_buffer_unref(&ctx->hw_frames_ctx);
             av_frame_side_data_free(&ctx->decoded_side_data,
@@ -878,6 +879,12 @@  static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
     if (!copy->internal->in_pkt)
         return AVERROR(ENOMEM);
 
+    if (avctx->internal->lcevc) {
+        copy->internal->lcevc = av_buffer_ref(avctx->internal->lcevc);
+        if (!copy->internal->lcevc)
+            return AVERROR(ENOMEM);
+    }
+
     copy->internal->last_pkt_props = av_packet_alloc();
     if (!copy->internal->last_pkt_props)
         return AVERROR(ENOMEM);