[FFmpeg-devel] [PATCH 1/3] lavc/speedhqdec: Add AV_CODEC_CAP_FRAME_THREADS

[FFmpeg-devel] [PATCH 1/3] lavc/speedhqdec: Add AV_CODEC_CAP_FRAME_THREADS

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 310 bytes --]

Hi

On a 36-core machine (Intel(R) Xeon(R) Platinum 8124M CPU @ 3.00GHz)
with a 7 minute 125 Mbit/s 1080p sample and -thread_type frame -threads
36 this brings CPU utilization from 117% to 3174%, 58x realtime

Without -threads 36 the utilization is only 1601% due to it being 16 by
default..

/Tomas

[-- Attachment #2: 0001-lavc-speedhqdec-Add-AV_CODEC_CAP_FRAME_THREADS.patch --]
[-- Type: text/x-patch, Size: 1327 bytes --]

From d1a7f33a1c394142b6dc14d4e8392ad786f8ba52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Wed, 8 May 2024 14:17:18 +0200
Subject: [PATCH 1/3] lavc/speedhqdec: Add AV_CODEC_CAP_FRAME_THREADS

---
 libavcodec/speedhqdec.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavcodec/speedhqdec.c b/libavcodec/speedhqdec.c
index d3605b0649..e54e25cb25 100644
--- a/libavcodec/speedhqdec.c
+++ b/libavcodec/speedhqdec.c
@@ -40,6 +40,7 @@
 #include "mpeg12data.h"
 #include "mpeg12vlc.h"
 #include "speedhq.h"
+#include "thread.h"
 
 #define MAX_INDEX (64 - 1)
 
@@ -433,7 +434,7 @@ static int speedhq_decode_frame(AVCodecContext *avctx, AVFrame *frame,
     avctx->coded_width = FFALIGN(avctx->width, 16);
     avctx->coded_height = FFALIGN(avctx->height, 16);
 
-    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) {
+    if ((ret = ff_thread_get_buffer(avctx, frame, 0)) < 0) {
         return ret;
     }
     frame->flags |= AV_FRAME_FLAG_KEY;
@@ -649,5 +650,5 @@ const FFCodec ff_speedhq_decoder = {
     .priv_data_size = sizeof(SHQContext),
     .init           = speedhq_decode_init,
     FF_CODEC_DECODE_CB(speedhq_decode_frame),
-    .p.capabilities = AV_CODEC_CAP_DR1,
+    .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_FRAME_THREADS,
 };
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/3] lavc/speedhqdec: Obey AVDISCARD_ALL

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 27 bytes --]

Inspired by qoidec

/Tomas

[-- Attachment #2: 0002-lavc-speedhqdec-Obey-AVDISCARD_ALL.patch --]
[-- Type: text/x-patch, Size: 782 bytes --]

From ed49d383d40a9a4fad04b76bc7d86107c6350d9a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Wed, 8 May 2024 14:17:57 +0200
Subject: [PATCH 2/3] lavc/speedhqdec: Obey AVDISCARD_ALL

---
 libavcodec/speedhqdec.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libavcodec/speedhqdec.c b/libavcodec/speedhqdec.c
index e54e25cb25..d6b1fff7a5 100644
--- a/libavcodec/speedhqdec.c
+++ b/libavcodec/speedhqdec.c
@@ -424,6 +424,9 @@ static int speedhq_decode_frame(AVCodecContext *avctx, AVFrame *frame,
         return AVERROR_INVALIDDATA;
     }
 
+    if (avctx->skip_frame >= AVDISCARD_ALL)
+        return avpkt->size;
+
     compute_quant_matrix(s->quant_matrix, 100 - quality);
 
     second_field_offset = AV_RL24(buf + 1);
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

[Tomas Härdin]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: 0003-lavc-speedhqdec-Set-AV_PICTURE_TYPE_I.patch --]
[-- Type: text/x-patch, Size: 755 bytes --]

From 894a0ff35e892f84b079bef62efa56250bb33e41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomas=20H=C3=A4rdin?= <git@haerdin.se>
Date: Wed, 8 May 2024 14:18:10 +0200
Subject: [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

---
 libavcodec/speedhqdec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavcodec/speedhqdec.c b/libavcodec/speedhqdec.c
index d6b1fff7a5..2244d73c15 100644
--- a/libavcodec/speedhqdec.c
+++ b/libavcodec/speedhqdec.c
@@ -441,6 +441,7 @@ static int speedhq_decode_frame(AVCodecContext *avctx, AVFrame *frame,
         return ret;
     }
     frame->flags |= AV_FRAME_FLAG_KEY;
+    frame->pict_type = AV_PICTURE_TYPE_I;
 
     if (second_field_offset == 4 || second_field_offset == (buf_size-4)) {
         /*
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

[Marton Balint]

On Wed, 8 May 2024, Tomas Härdin wrote:

>
>

What suprises me is that pict_type and the keyframe flag is not set 
already for decoding codecs with AV_CODEC_PROP_INTRA_ONLY flag. Is this 
intentional or just nobody had the time to set it up to work 
automatically?

Thanks,
Marton
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

[James Almer]

On 5/8/2024 5:01 PM, Marton Balint wrote:
> 
> 
> On Wed, 8 May 2024, Tomas Härdin wrote:
> 
>>
>>
> 
> What suprises me is that pict_type and the keyframe flag is not set 
> already for decoding codecs with AV_CODEC_PROP_INTRA_ONLY flag. Is this 
> intentional or just nobody had the time to set it up to work automatically?

For audio it's not always the case (See MLP/TrueHD). For video it might 
with intra-only codecs, but at least with non intra-only codecs, an I 
frame is not necessarily a keyframe (See AV1).

> 
> Thanks,
> Marton
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

[Tomas Härdin]

ons 2024-05-08 klockan 17:06 -0300 skrev James Almer:
> On 5/8/2024 5:01 PM, Marton Balint wrote:
> > 
> > 
> > On Wed, 8 May 2024, Tomas Härdin wrote:
> > 
> > > 
> > > 
> > 
> > What suprises me is that pict_type and the keyframe flag is not set
> > already for decoding codecs with AV_CODEC_PROP_INTRA_ONLY flag. Is
> > this 
> > intentional or just nobody had the time to set it up to work
> > automatically?
> 
> For audio it's not always the case (See MLP/TrueHD). For video it
> might 
> with intra-only codecs, but at least with non intra-only codecs, an I
> frame is not necessarily a keyframe (See AV1).

Yes, IDR frames exist. We're talking about intra-only though.

I'll push this on Monday if there are no objections. We can adds some
logic for always setting key_frame and/or pict_type in the appropriate
place for intra-only codecs as a separate patch. That would allow us to
simplify all intra-only codecs as well, which would be nice

/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

[Andreas Rheinhardt]

Tomas Härdin:
> ons 2024-05-08 klockan 17:06 -0300 skrev James Almer:
>> On 5/8/2024 5:01 PM, Marton Balint wrote:
>>>
>>>
>>> On Wed, 8 May 2024, Tomas Härdin wrote:
>>>
>>>>
>>>>
>>>
>>> What suprises me is that pict_type and the keyframe flag is not set
>>> already for decoding codecs with AV_CODEC_PROP_INTRA_ONLY flag. Is
>>> this 
>>> intentional or just nobody had the time to set it up to work
>>> automatically?
>>
>> For audio it's not always the case (See MLP/TrueHD). For video it
>> might 
>> with intra-only codecs, but at least with non intra-only codecs, an I
>> frame is not necessarily a keyframe (See AV1).
> 
> Yes, IDR frames exist. We're talking about intra-only though.
> 
> I'll push this on Monday if there are no objections. We can adds some
> logic for always setting key_frame and/or pict_type in the appropriate
> place for intra-only codecs as a separate patch. That would allow us to
> simplify all intra-only codecs as well, which would be nice
> 

Already done: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/327065.html

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

[Tomas Härdin]

fre 2024-05-10 klockan 14:48 +0200 skrev Andreas Rheinhardt:
> Tomas Härdin:
> > ons 2024-05-08 klockan 17:06 -0300 skrev James Almer:
> > > On 5/8/2024 5:01 PM, Marton Balint wrote:
> > > > 
> > > > 
> > > > On Wed, 8 May 2024, Tomas Härdin wrote:
> > > > 
> > > > > 
> > > > > 
> > > > 
> > > > What suprises me is that pict_type and the keyframe flag is not
> > > > set
> > > > already for decoding codecs with AV_CODEC_PROP_INTRA_ONLY flag.
> > > > Is
> > > > this 
> > > > intentional or just nobody had the time to set it up to work
> > > > automatically?
> > > 
> > > For audio it's not always the case (See MLP/TrueHD). For video it
> > > might 
> > > with intra-only codecs, but at least with non intra-only codecs,
> > > an I
> > > frame is not necessarily a keyframe (See AV1).
> > 
> > Yes, IDR frames exist. We're talking about intra-only though.
> > 
> > I'll push this on Monday if there are no objections. We can adds
> > some
> > logic for always setting key_frame and/or pict_type in the
> > appropriate
> > place for intra-only codecs as a separate patch. That would allow
> > us to
> > simplify all intra-only codecs as well, which would be nice
> > 
> 
> Already done:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/327065.html

Cool. I'll not bother with this patch then.

/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] lavc/speedhqdec: Set AV_PICTURE_TYPE_I

[Tomas Härdin]

sön 2024-05-12 klockan 01:07 +0200 skrev Tomas Härdin:
> fre 2024-05-10 klockan 14:48 +0200 skrev Andreas Rheinhardt:
> > Tomas Härdin:
> > > ons 2024-05-08 klockan 17:06 -0300 skrev James Almer:
> > > > On 5/8/2024 5:01 PM, Marton Balint wrote:
> > > > > 
> > > > > 
> > > > > On Wed, 8 May 2024, Tomas Härdin wrote:
> > > > > 
> > > > > > 
> > > > > > 
> > > > > 
> > > > > What suprises me is that pict_type and the keyframe flag is
> > > > > not
> > > > > set
> > > > > already for decoding codecs with AV_CODEC_PROP_INTRA_ONLY
> > > > > flag.
> > > > > Is
> > > > > this 
> > > > > intentional or just nobody had the time to set it up to work
> > > > > automatically?
> > > > 
> > > > For audio it's not always the case (See MLP/TrueHD). For video
> > > > it
> > > > might 
> > > > with intra-only codecs, but at least with non intra-only
> > > > codecs,
> > > > an I
> > > > frame is not necessarily a keyframe (See AV1).
> > > 
> > > Yes, IDR frames exist. We're talking about intra-only though.
> > > 
> > > I'll push this on Monday if there are no objections. We can adds
> > > some
> > > logic for always setting key_frame and/or pict_type in the
> > > appropriate
> > > place for intra-only codecs as a separate patch. That would allow
> > > us to
> > > simplify all intra-only codecs as well, which would be nice
> > > 
> > 
> > Already done:
> > https://ffmpeg.org/pipermail/ffmpeg-devel/2024-May/327065.html
> 
> Cool. I'll not bother with this patch then.

Pushed patches 1 and 2

/Tomas
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] lavc/speedhqdec: Add AV_CODEC_CAP_FRAME_THREADS

[James Almer]

On 5/8/2024 9:42 AM, Tomas Härdin wrote:
> Hi
> 
> On a 36-core machine (Intel(R) Xeon(R) Platinum 8124M CPU @ 3.00GHz)
> with a 7 minute 125 Mbit/s 1080p sample and -thread_type frame -threads
> 36 this brings CPU utilization from 117% to 3174%, 58x realtime
> 
> Without -threads 36 the utilization is only 1601% due to it being 16 by
> default..

Does this default make sense anymore? I think it was added for h264, but 
maybe it should be decoder dependent.

> 
> /Tomas
> 
> 
> _______________________________________________
> 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".
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] lavc/speedhqdec: Add AV_CODEC_CAP_FRAME_THREADS

[Tomas Härdin]

ons 2024-05-08 klockan 09:46 -0300 skrev James Almer:
> On 5/8/2024 9:42 AM, Tomas Härdin wrote:
> > Hi
> > 
> > On a 36-core machine (Intel(R) Xeon(R) Platinum 8124M CPU @
> > 3.00GHz)
> > with a 7 minute 125 Mbit/s 1080p sample and -thread_type frame -
> > threads
> > 36 this brings CPU utilization from 117% to 3174%, 58x realtime
> > 
> > Without -threads 36 the utilization is only 1601% due to it being
> > 16 by
> > default..
> 
> Does this default make sense anymore? I think it was added for h264,
> but 
> maybe it should be decoder dependent.

It is only relevant for slice threading in h264, right? It certainly
doesn't make sense for any intra-only codec in frame threading mode

/Tomas
_______________________________________________
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".