* [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
@ 2023-05-20 1:50 James Almer
2023-05-20 16:12 ` Anton Khirnov
2023-05-20 17:23 ` Michael Niedermayer
0 siblings, 2 replies; 11+ messages in thread
From: James Almer @ 2023-05-20 1:50 UTC (permalink / raw)
To: ffmpeg-devel
Signed-off-by: James Almer <jamrial@gmail.com>
---
libavcodec/av1dec.c | 75 +++++++++++++++++++++++++++++++++------------
libavcodec/av1dec.h | 4 +++
2 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index d46ee48335..53ed37b817 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -32,6 +32,7 @@
#include "bytestream.h"
#include "codec_internal.h"
#include "decode.h"
+#include "internal.h"
#include "hwconfig.h"
#include "profiles.h"
#include "thread.h"
@@ -760,6 +761,7 @@ static const CodedBitstreamUnitType decompose_unit_types[] = {
static av_cold int av1_decode_init(AVCodecContext *avctx)
{
+ AVCodecInternal *avci = avctx->internal;
AV1DecContext *s = avctx->priv_data;
AV1RawSequenceHeader *seq;
int ret;
@@ -767,6 +769,8 @@ static av_cold int av1_decode_init(AVCodecContext *avctx)
s->avctx = avctx;
s->pix_fmt = AV_PIX_FMT_NONE;
+ s->pkt = avci->in_pkt;
+
for (int i = 0; i < FF_ARRAY_ELEMS(s->ref); i++) {
s->ref[i].f = av_frame_alloc();
if (!s->ref[i].f) {
@@ -1041,11 +1045,11 @@ static int export_film_grain(AVCodecContext *avctx, AVFrame *frame)
return 0;
}
-static int set_output_frame(AVCodecContext *avctx, AVFrame *frame,
- const AVPacket *pkt, int *got_frame)
+static int set_output_frame(AVCodecContext *avctx, AVFrame *frame)
{
AV1DecContext *s = avctx->priv_data;
const AVFrame *srcframe = s->cur_frame.f;
+ AVPacket *pkt = s->pkt;
int ret;
// TODO: all layers
@@ -1079,7 +1083,7 @@ FF_DISABLE_DEPRECATION_WARNINGS
FF_ENABLE_DEPRECATION_WARNINGS
#endif
- *got_frame = 1;
+ av_packet_unref(pkt);
return 0;
}
@@ -1145,22 +1149,32 @@ static int get_current_frame(AVCodecContext *avctx)
return ret;
}
-static int av1_decode_frame(AVCodecContext *avctx, AVFrame *frame,
- int *got_frame, AVPacket *pkt)
+static int av1_receive_frame(AVCodecContext *avctx, AVFrame *frame)
{
AV1DecContext *s = avctx->priv_data;
AV1RawTileGroup *raw_tile_group = NULL;
- int ret;
+ int i, ret;
- ret = ff_cbs_read_packet(s->cbc, &s->current_obu, pkt);
- if (ret < 0) {
- av_log(avctx, AV_LOG_ERROR, "Failed to read packet.\n");
- goto end;
+again:
+ if (!s->current_obu.nb_units) {
+ av_packet_unref(s->pkt);
+ ret = ff_decode_get_packet(avctx, s->pkt);
+ if (ret < 0)
+ return ret;
+
+ ret = ff_cbs_read_packet(s->cbc, &s->current_obu, s->pkt);
+ if (ret < 0) {
+ av_log(avctx, AV_LOG_ERROR, "Failed to read packet.\n");
+ goto end;
+ }
+
+ s->nb_unit = 0;
+
+ av_log(avctx, AV_LOG_DEBUG, "Total OBUs on this packet:%d.\n",
+ s->current_obu.nb_units);
}
- av_log(avctx, AV_LOG_DEBUG, "Total obu for this frame:%d.\n",
- s->current_obu.nb_units);
- for (int i = 0; i < s->current_obu.nb_units; i++) {
+ for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
CodedBitstreamUnit *unit = &s->current_obu.units[i];
AV1RawOBU *obu = unit->content;
const AV1RawOBUHeader *header;
@@ -1168,6 +1182,7 @@ static int av1_decode_frame(AVCodecContext *avctx, AVFrame *frame,
if (!obu)
continue;
+ ret = 0;
header = &obu->header;
av_log(avctx, AV_LOG_DEBUG, "Obu idx:%d, obu type:%d.\n", i, unit->type);
@@ -1251,13 +1266,15 @@ static int av1_decode_frame(AVCodecContext *avctx, AVFrame *frame,
goto end;
}
+ ret = 0;
if (s->cur_frame.f->buf[0]) {
- ret = set_output_frame(avctx, frame, pkt, got_frame);
+ ret = set_output_frame(avctx, frame);
if (ret < 0)
av_log(avctx, AV_LOG_ERROR, "Set output frame error.\n");
}
s->raw_frame_header = NULL;
+ i++;
goto end;
}
@@ -1361,6 +1378,7 @@ static int av1_decode_frame(AVCodecContext *avctx, AVFrame *frame,
}
if (raw_tile_group && (s->tile_num == raw_tile_group->tg_end + 1)) {
+ int show_frame = s->raw_frame_header->show_frame;
if (avctx->hwaccel && s->cur_frame.f->buf[0]) {
ret = avctx->hwaccel->end_frame(avctx);
if (ret < 0) {
@@ -1375,8 +1393,9 @@ static int av1_decode_frame(AVCodecContext *avctx, AVFrame *frame,
goto end;
}
+ ret = 0;
if (s->raw_frame_header->show_frame && s->cur_frame.f->buf[0]) {
- ret = set_output_frame(avctx, frame, pkt, got_frame);
+ ret = set_output_frame(avctx, frame);
if (ret < 0) {
av_log(avctx, AV_LOG_ERROR, "Set output frame error\n");
goto end;
@@ -1384,13 +1403,30 @@ static int av1_decode_frame(AVCodecContext *avctx, AVFrame *frame,
}
raw_tile_group = NULL;
s->raw_frame_header = NULL;
+ if (show_frame) {
+ i++;
+ goto end;
+ }
}
+ ret = AVERROR(EAGAIN);
}
end:
- ff_cbs_fragment_reset(&s->current_obu);
- if (ret < 0)
+ av_assert0(i <= s->current_obu.nb_units);
+ s->nb_unit = i;
+
+ if (s->current_obu.nb_units == i) {
+ ff_cbs_fragment_reset(&s->current_obu);
+ s->nb_unit = 0;
+ }
+ if (ret == AVERROR(EAGAIN))
+ goto again;
+ if (ret < 0) {
s->raw_frame_header = NULL;
+ ff_cbs_fragment_reset(&s->current_obu);
+ s->nb_unit = 0;
+ }
+
return ret;
}
@@ -1404,6 +1440,7 @@ static void av1_decode_flush(AVCodecContext *avctx)
av1_frame_unref(avctx, &s->cur_frame);
s->operating_point_idc = 0;
+ s->nb_unit = 0;
s->raw_frame_header = NULL;
s->raw_seq = NULL;
s->cll = NULL;
@@ -1411,6 +1448,7 @@ static void av1_decode_flush(AVCodecContext *avctx)
while (av_fifo_read(s->itut_t35_fifo, &itut_t35, 1) >= 0)
av_buffer_unref(&itut_t35.payload_ref);
+ ff_cbs_fragment_reset(&s->current_obu);
ff_cbs_flush(s->cbc);
}
@@ -1437,14 +1475,13 @@ const FFCodec ff_av1_decoder = {
.priv_data_size = sizeof(AV1DecContext),
.init = av1_decode_init,
.close = av1_decode_free,
- FF_CODEC_DECODE_CB(av1_decode_frame),
+ FF_CODEC_RECEIVE_FRAME_CB(av1_receive_frame),
.p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_AVOID_PROBING,
.caps_internal = FF_CODEC_CAP_INIT_CLEANUP |
FF_CODEC_CAP_SETS_PKT_DTS,
.flush = av1_decode_flush,
.p.profiles = NULL_IF_CONFIG_SMALL(ff_av1_profiles),
.p.priv_class = &av1_class,
- .bsfs = "av1_frame_split",
.hw_configs = (const AVCodecHWConfigInternal *const []) {
#if CONFIG_AV1_DXVA2_HWACCEL
HWACCEL_DXVA2(av1),
diff --git a/libavcodec/av1dec.h b/libavcodec/av1dec.h
index cef899f81f..59ffed1d9b 100644
--- a/libavcodec/av1dec.h
+++ b/libavcodec/av1dec.h
@@ -28,6 +28,7 @@
#include "libavutil/frame.h"
#include "libavutil/pixfmt.h"
#include "avcodec.h"
+#include "packet.h"
#include "cbs.h"
#include "cbs_av1.h"
@@ -68,6 +69,7 @@ typedef struct AV1DecContext {
enum AVPixelFormat pix_fmt;
CodedBitstreamContext *cbc;
CodedBitstreamFragment current_obu;
+ AVPacket *pkt;
AVBufferRef *seq_ref;
AV1RawSequenceHeader *raw_seq;
@@ -90,6 +92,8 @@ typedef struct AV1DecContext {
AV1Frame ref[AV1_NUM_REF_FRAMES];
AV1Frame cur_frame;
+ int nb_unit;
+
// AVOptions
int operating_point;
} AV1DecContext;
--
2.40.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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 1:50 [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame() James Almer
@ 2023-05-20 16:12 ` Anton Khirnov
2023-05-20 16:44 ` James Almer
2023-05-20 17:23 ` Michael Niedermayer
1 sibling, 1 reply; 11+ messages in thread
From: Anton Khirnov @ 2023-05-20 16:12 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Quoting James Almer (2023-05-20 03:50:57)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/av1dec.c | 75 +++++++++++++++++++++++++++++++++------------
> libavcodec/av1dec.h | 4 +++
> 2 files changed, 60 insertions(+), 19 deletions(-)
The patch makes the code longer and introduces an evil backward goto.
So some comment on why is this an improvement would be appropriate.
--
Anton Khirnov
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 16:12 ` Anton Khirnov
@ 2023-05-20 16:44 ` James Almer
2023-05-20 16:59 ` Andreas Rheinhardt
0 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2023-05-20 16:44 UTC (permalink / raw)
To: ffmpeg-devel
On 5/20/2023 1:12 PM, Anton Khirnov wrote:
> Quoting James Almer (2023-05-20 03:50:57)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavcodec/av1dec.c | 75 +++++++++++++++++++++++++++++++++------------
>> libavcodec/av1dec.h | 4 +++
>> 2 files changed, 60 insertions(+), 19 deletions(-)
>
> The patch makes the code longer and introduces an evil backward goto.
> So some comment on why is this an improvement would be appropriate.
It's an improvement because it removes the auto-inserted av1_frame_split
from the process, making the decoder handle temporal units with more
than one frame in them on its own. I can add that to the commit message.
The extra code is inevitable because it's the decoder who now needs to
fetch the packet instead of the generic code in decode.c
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 16:44 ` James Almer
@ 2023-05-20 16:59 ` Andreas Rheinhardt
2023-05-20 17:01 ` James Almer
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2023-05-20 16:59 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> On 5/20/2023 1:12 PM, Anton Khirnov wrote:
>> Quoting James Almer (2023-05-20 03:50:57)
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>> libavcodec/av1dec.c | 75 +++++++++++++++++++++++++++++++++------------
>>> libavcodec/av1dec.h | 4 +++
>>> 2 files changed, 60 insertions(+), 19 deletions(-)
>>
>> The patch makes the code longer and introduces an evil backward goto.
>> So some comment on why is this an improvement would be appropriate.
>
> It's an improvement because it removes the auto-inserted av1_frame_split
> from the process, making the decoder handle temporal units with more
> than one frame in them on its own. I can add that to the commit message.
>
> The extra code is inevitable because it's the decoder who now needs to
> fetch the packet instead of the generic code in decode.c
1. Where is the improvement in that? What is so bad about using a BSF to
preprocess packets in this way?
2. Anyway, you did not remove the decoder->bsf configure dependency.
- 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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 16:59 ` Andreas Rheinhardt
@ 2023-05-20 17:01 ` James Almer
2023-05-20 17:07 ` Andreas Rheinhardt
0 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2023-05-20 17:01 UTC (permalink / raw)
To: ffmpeg-devel
On 5/20/2023 1:59 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 5/20/2023 1:12 PM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-05-20 03:50:57)
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>> libavcodec/av1dec.c | 75 +++++++++++++++++++++++++++++++++------------
>>>> libavcodec/av1dec.h | 4 +++
>>>> 2 files changed, 60 insertions(+), 19 deletions(-)
>>>
>>> The patch makes the code longer and introduces an evil backward goto.
>>> So some comment on why is this an improvement would be appropriate.
>>
>> It's an improvement because it removes the auto-inserted av1_frame_split
>> from the process, making the decoder handle temporal units with more
>> than one frame in them on its own. I can add that to the commit message.
>>
>> The extra code is inevitable because it's the decoder who now needs to
>> fetch the packet instead of the generic code in decode.c
>
> 1. Where is the improvement in that? What is so bad about using a BSF to
> preprocess packets in this way?
Much less overhead? One less instance of CBS, less function calls, less
packet references generated and moved around, etc.
> 2. Anyway, you did not remove the decoder->bsf configure dependency.
Will amend.
>
> - 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".
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 17:01 ` James Almer
@ 2023-05-20 17:07 ` Andreas Rheinhardt
2023-05-20 17:15 ` James Almer
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2023-05-20 17:07 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> On 5/20/2023 1:59 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 5/20/2023 1:12 PM, Anton Khirnov wrote:
>>>> Quoting James Almer (2023-05-20 03:50:57)
>>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>>> ---
>>>>> libavcodec/av1dec.c | 75
>>>>> +++++++++++++++++++++++++++++++++------------
>>>>> libavcodec/av1dec.h | 4 +++
>>>>> 2 files changed, 60 insertions(+), 19 deletions(-)
>>>>
>>>> The patch makes the code longer and introduces an evil backward goto.
>>>> So some comment on why is this an improvement would be appropriate.
>>>
>>> It's an improvement because it removes the auto-inserted av1_frame_split
>>> from the process, making the decoder handle temporal units with more
>>> than one frame in them on its own. I can add that to the commit message.
>>>
>>> The extra code is inevitable because it's the decoder who now needs to
>>> fetch the packet instead of the generic code in decode.c
>>
>> 1. Where is the improvement in that? What is so bad about using a BSF to
>> preprocess packets in this way?
>
> Much less overhead? One less instance of CBS, less function calls, less
> packet references generated and moved around, etc.
>
This is AV1, not annex B H.2645.
- 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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 17:07 ` Andreas Rheinhardt
@ 2023-05-20 17:15 ` James Almer
2023-05-20 17:17 ` Andreas Rheinhardt
0 siblings, 1 reply; 11+ messages in thread
From: James Almer @ 2023-05-20 17:15 UTC (permalink / raw)
To: ffmpeg-devel
On 5/20/2023 2:07 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 5/20/2023 1:59 PM, Andreas Rheinhardt wrote:
>>> James Almer:
>>>> On 5/20/2023 1:12 PM, Anton Khirnov wrote:
>>>>> Quoting James Almer (2023-05-20 03:50:57)
>>>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>>>> ---
>>>>>> libavcodec/av1dec.c | 75
>>>>>> +++++++++++++++++++++++++++++++++------------
>>>>>> libavcodec/av1dec.h | 4 +++
>>>>>> 2 files changed, 60 insertions(+), 19 deletions(-)
>>>>>
>>>>> The patch makes the code longer and introduces an evil backward goto.
>>>>> So some comment on why is this an improvement would be appropriate.
>>>>
>>>> It's an improvement because it removes the auto-inserted av1_frame_split
>>>> from the process, making the decoder handle temporal units with more
>>>> than one frame in them on its own. I can add that to the commit message.
>>>>
>>>> The extra code is inevitable because it's the decoder who now needs to
>>>> fetch the packet instead of the generic code in decode.c
>>>
>>> 1. Where is the improvement in that? What is so bad about using a BSF to
>>> preprocess packets in this way?
>>
>> Much less overhead? One less instance of CBS, less function calls, less
>> packet references generated and moved around, etc.
>>
>
> This is AV1, not annex B H.2645.
I'm sorry, i don't understand what you're trying to say. The
av1_frame_split bsf will split a TU into individual frames, and uses the
CBS framework for this. It's an unnecessary overhead when the decoder
can be made to handle this on its own by making it use the proper decode
callback.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 17:15 ` James Almer
@ 2023-05-20 17:17 ` Andreas Rheinhardt
2023-05-20 17:20 ` James Almer
0 siblings, 1 reply; 11+ messages in thread
From: Andreas Rheinhardt @ 2023-05-20 17:17 UTC (permalink / raw)
To: ffmpeg-devel
James Almer:
> On 5/20/2023 2:07 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 5/20/2023 1:59 PM, Andreas Rheinhardt wrote:
>>>> James Almer:
>>>>> On 5/20/2023 1:12 PM, Anton Khirnov wrote:
>>>>>> Quoting James Almer (2023-05-20 03:50:57)
>>>>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>>>>> ---
>>>>>>> libavcodec/av1dec.c | 75
>>>>>>> +++++++++++++++++++++++++++++++++------------
>>>>>>> libavcodec/av1dec.h | 4 +++
>>>>>>> 2 files changed, 60 insertions(+), 19 deletions(-)
>>>>>>
>>>>>> The patch makes the code longer and introduces an evil backward goto.
>>>>>> So some comment on why is this an improvement would be appropriate.
>>>>>
>>>>> It's an improvement because it removes the auto-inserted
>>>>> av1_frame_split
>>>>> from the process, making the decoder handle temporal units with more
>>>>> than one frame in them on its own. I can add that to the commit
>>>>> message.
>>>>>
>>>>> The extra code is inevitable because it's the decoder who now needs to
>>>>> fetch the packet instead of the generic code in decode.c
>>>>
>>>> 1. Where is the improvement in that? What is so bad about using a
>>>> BSF to
>>>> preprocess packets in this way?
>>>
>>> Much less overhead? One less instance of CBS, less function calls, less
>>> packet references generated and moved around, etc.
>>>
>>
>> This is AV1, not annex B H.2645.
>
> I'm sorry, i don't understand what you're trying to say. The
> av1_frame_split bsf will split a TU into individual frames, and uses the
> CBS framework for this. It's an unnecessary overhead when the decoder
> can be made to handle this on its own by making it use the proper decode
> callback.
I meant to say: Parsing AV1 is easy and fast, in contrast to H.2645 (in
particular, annex B H.2645).
- 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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 17:17 ` Andreas Rheinhardt
@ 2023-05-20 17:20 ` James Almer
0 siblings, 0 replies; 11+ messages in thread
From: James Almer @ 2023-05-20 17:20 UTC (permalink / raw)
To: ffmpeg-devel
On 5/20/2023 2:17 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 5/20/2023 2:07 PM, Andreas Rheinhardt wrote:
>>> James Almer:
>>>> On 5/20/2023 1:59 PM, Andreas Rheinhardt wrote:
>>>>> James Almer:
>>>>>> On 5/20/2023 1:12 PM, Anton Khirnov wrote:
>>>>>>> Quoting James Almer (2023-05-20 03:50:57)
>>>>>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>>>>>> ---
>>>>>>>> libavcodec/av1dec.c | 75
>>>>>>>> +++++++++++++++++++++++++++++++++------------
>>>>>>>> libavcodec/av1dec.h | 4 +++
>>>>>>>> 2 files changed, 60 insertions(+), 19 deletions(-)
>>>>>>>
>>>>>>> The patch makes the code longer and introduces an evil backward goto.
>>>>>>> So some comment on why is this an improvement would be appropriate.
>>>>>>
>>>>>> It's an improvement because it removes the auto-inserted
>>>>>> av1_frame_split
>>>>>> from the process, making the decoder handle temporal units with more
>>>>>> than one frame in them on its own. I can add that to the commit
>>>>>> message.
>>>>>>
>>>>>> The extra code is inevitable because it's the decoder who now needs to
>>>>>> fetch the packet instead of the generic code in decode.c
>>>>>
>>>>> 1. Where is the improvement in that? What is so bad about using a
>>>>> BSF to
>>>>> preprocess packets in this way?
>>>>
>>>> Much less overhead? One less instance of CBS, less function calls, less
>>>> packet references generated and moved around, etc.
>>>>
>>>
>>> This is AV1, not annex B H.2645.
>>
>> I'm sorry, i don't understand what you're trying to say. The
>> av1_frame_split bsf will split a TU into individual frames, and uses the
>> CBS framework for this. It's an unnecessary overhead when the decoder
>> can be made to handle this on its own by making it use the proper decode
>> callback.
>
> I meant to say: Parsing AV1 is easy and fast, in contrast to H.2645 (in
> particular, annex B H.2645).
Sure, but it's still an unnecessary overhead. Right now, the parser
spawns a CBS instance, and the decoder two. I'm removing the one that's
superfluous.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 1:50 [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame() James Almer
2023-05-20 16:12 ` Anton Khirnov
@ 2023-05-20 17:23 ` Michael Niedermayer
2023-05-20 17:43 ` James Almer
1 sibling, 1 reply; 11+ messages in thread
From: Michael Niedermayer @ 2023-05-20 17:23 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1732 bytes --]
On Fri, May 19, 2023 at 10:50:57PM -0300, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> libavcodec/av1dec.c | 75 +++++++++++++++++++++++++++++++++------------
> libavcodec/av1dec.h | 4 +++
> 2 files changed, 60 insertions(+), 19 deletions(-)
Crashes intermittently (so maybe its not this one)
[av1 @ 0x562d7e65db40] No sequence header available.
[av1 @ 0x562d7e65db40] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment.
[av1 @ 0x562d7e65db40] Failed to parse temporal unit.
[av1 @ 0x562d7e65db40] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment.
[av1 @ 0x562d7e65db40] Failed to read packet.
Assertion i <= s->current_obu.nb_units failed at libavcodec/av1dec.c:1415
Aborted (core dumped)
#0 0x00007fffed2e2e87 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1 0x00007fffed2e47f1 in __GI_abort () at abort.c:79
#2 0x0000555555bfcf4a in av1_receive_frame ()
#3 0x0000555555c7d814 in decode_receive_frame_internal ()
#4 0x0000555555c7e4c0 in avcodec_send_packet ()
#5 0x0000555555a5d633 in try_decode_frame ()
#6 0x0000555555a628ff in avformat_find_stream_info ()
#7 0x000055555574578e in ifile_open ()
#8 0x000055555575c99c in open_files.isra ()
#9 0x000055555575ddb5 in ffmpeg_parse_options ()
#10 0x000055555573df61 in main ()
if i build with full debug symbols it doesnt crash :)
ill mail you the file
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: 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".
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame()
2023-05-20 17:23 ` Michael Niedermayer
@ 2023-05-20 17:43 ` James Almer
0 siblings, 0 replies; 11+ messages in thread
From: James Almer @ 2023-05-20 17:43 UTC (permalink / raw)
To: ffmpeg-devel
On 5/20/2023 2:23 PM, Michael Niedermayer wrote:
> On Fri, May 19, 2023 at 10:50:57PM -0300, James Almer wrote:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> libavcodec/av1dec.c | 75 +++++++++++++++++++++++++++++++++------------
>> libavcodec/av1dec.h | 4 +++
>> 2 files changed, 60 insertions(+), 19 deletions(-)
>
> Crashes intermittently (so maybe its not this one)
>
> [av1 @ 0x562d7e65db40] No sequence header available.
> [av1 @ 0x562d7e65db40] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment.
> [av1 @ 0x562d7e65db40] Failed to parse temporal unit.
> [av1 @ 0x562d7e65db40] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment.
> [av1 @ 0x562d7e65db40] Failed to read packet.
> Assertion i <= s->current_obu.nb_units failed at libavcodec/av1dec.c:1415
> Aborted (core dumped)
>
> #0 0x00007fffed2e2e87 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
> #1 0x00007fffed2e47f1 in __GI_abort () at abort.c:79
> #2 0x0000555555bfcf4a in av1_receive_frame ()
> #3 0x0000555555c7d814 in decode_receive_frame_internal ()
> #4 0x0000555555c7e4c0 in avcodec_send_packet ()
> #5 0x0000555555a5d633 in try_decode_frame ()
> #6 0x0000555555a628ff in avformat_find_stream_info ()
> #7 0x000055555574578e in ifile_open ()
> #8 0x000055555575c99c in open_files.isra ()
> #9 0x000055555575ddb5 in ffmpeg_parse_options ()
> #10 0x000055555573df61 in main ()
>
> if i build with full debug symbols it doesnt crash :)
> ill mail you the file
"i" was uninitialized because the for loop was never reached. Fixed locally.
>
>
> [...]
>
>
> _______________________________________________
> 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".
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-20 17:43 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-20 1:50 [FFmpeg-devel] [PATCH v2] avcodec/av1dec: convert to receive_frame() James Almer
2023-05-20 16:12 ` Anton Khirnov
2023-05-20 16:44 ` James Almer
2023-05-20 16:59 ` Andreas Rheinhardt
2023-05-20 17:01 ` James Almer
2023-05-20 17:07 ` Andreas Rheinhardt
2023-05-20 17:15 ` James Almer
2023-05-20 17:17 ` Andreas Rheinhardt
2023-05-20 17:20 ` James Almer
2023-05-20 17:23 ` Michael Niedermayer
2023-05-20 17:43 ` James Almer
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git