Re: [FFmpeg-devel] [PATCH 2/2] avcodec/videotoolbox: av1 decoding not copying the sequence header obu into the bitstream

2024-05-12 Thread Ruslan Chernenko
(resent due to not adding mail-list into the CC)
Hey there!
Thanks for checking out the patch;

As for videotoolbox_av1_start_frame:
For the av_fast_realloc call it's mostly the same thing as
ff_videotoolbox_buffer_copy defined at libavcodec/videotoolbox.c:79. It
takes a pointer to a bitstream, its allocated size and updates it. Also
there is a videotoolbox_common_decode_slice defined at libavcodec/
videotoolbox:434, which does a pretty similar job to the one I mentioned,
but a little different.

As for videotoolbox_av1_decode_params:
vtctx->bitstream at that moment shall be empty at that point, so does the
vtctx->bitsream_size
And as it turned out the sequence header obu needs to be copied into the
bitstream for videotoolbox to decode a frame as mentioned in the thread on
the apple forums(check the last but one comment)[1]

So basically what's happening here:
videotoolbox_av1_decode_params getting called the first in that decoding
loop at libavcodec/av1dec.c:1243, it copies the sequence header obu into
the bitstream as the first bytes(because at that point bitstream shall be
zero), and increases the vtctx->bitstream_size by the size of the sequence
header obu.
Then videotoolbox_av1_start_frame is getting called at
libavcodec/av1dec.c:1364 and copies the frame itself into the
vtctx->bitstream, taking into account the shift needed by the
vtctx->bitstream_size(which at that moment should be the size of the
sequence header obu).

[1] https://forums.developer.apple.com/forums/thread/739953


On Sun, 12 May 2024 at 11:52, Ruslan Chernenko  wrote:

> Hey there!
> Thanks for checking out the patch;
>
> As for videotoolbox_av1_start_frame:
> For the av_fast_realloc call it's mostly the same thing as
> ff_videotoolbox_buffer_copy defined at libavcodec/videotoolbox.c:79. It
> takes a pointer to a bitstream, its allocated size and updates it. Also
> there is a videotoolbox_common_decode_slice defined at
> libavcodec/videotoolbox:434, which does a pretty similar job to the one I
> mentioned, but a little different.
>
> As for videotoolbox_av1_decode_params:
> vtctx->bitstream at that moment shall be empty at that point, so does the
> vtctx->bitsream_size
> And as it turned out the sequence header obu needs to be copied into the
> bitstream for videotoolbox to decode a frame as mentioned in the thread on
> the apple forums(check the last but one comment)[1]
>
> So basically what's happening here:
> videotoolbox_av1_decode_params getting called the first in that decoding
> loop at libavcodec/av1dec.c:1243, it copies the sequence header obu into
> the bitstream as the first bytes(because at that point bitstream shall be
> zero), and increases the vtctx->bitstream_size by the size of the sequence
> header obu.
> Then videotoolbox_av1_start_frame is getting called at
> libavcodec/av1dec.c:1364 and copies the frame itself into the
> vtctx->bitstream, taking into account the shift needed by the
> vtctx->bitstream_size(which at that moment should be the size of the
> sequence header obu).
>
> [1] https://forums.developer.apple.com/forums/thread/739953
>
>
> On Sun, 12 May 2024 at 04:52, Cameron Gutman 
> wrote:
>
>> On Mon, May 6, 2024 at 3:45 PM Черненко Руслан 
>> wrote:
>> >
>> > Signed-off-by: Chernenko Ruslan 
>> > ---
>> >   libavcodec/videotoolbox_av1.c | 102 --
>> >   1 file changed, 72 insertions(+), 30 deletions(-)
>> >
>> > diff --git a/libavcodec/videotoolbox_av1.c
>> b/libavcodec/videotoolbox_av1.c
>> > index 7f7270c466..736f2548db 100644
>> > --- a/libavcodec/videotoolbox_av1.c
>> > +++ b/libavcodec/videotoolbox_av1.c
>> > @@ -19,50 +19,90 @@
>> >* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> > 02110-1301 USA
>> >*/
>> >   -#include "libavformat/avio.h"
>> > -#include "libavformat/avio_internal.h"
>> > -#define CALLED_FROM_AVCODEC 1
>> > -#include "libavformat/av1.c"
>> > -#undef CALLED_FROM_AVCODEC
>> >   -#include "libavutil/avutil.h"
>> > -#include "libavutil/frame.h"
>> > -#include "libavutil/pixfmt.h"
>> > +#include "libavutil/mem.h"
>> >#include "av1dec.h"
>> > -#include "codec_id.h"
>> >   #include "hwaccel_internal.h"
>> >   #include "internal.h"
>> >   #include "vt_internal.h"
>> >   -CFDataRef ff_videotoolbox_av1c_extradata_create(AVCodecContext
>> *avctx)
>> > +
>> > +CFDataRef ff_videotoolbox_av1c_extradata_create(AVCodecContext *avctx)
>> {
>> >

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/videotoolbox: av1 decoding not copying the sequence header obu into the bitstream

2024-05-12 Thread Ruslan Chernenko
Okay, yeah.
Took a better look and indeed it should be (vtctx->bitstream_size + size)
for the av_fast_realloc.
Will send an update on this patch today.

On Sun, 12 May 2024 at 11:53, Ruslan Chernenko  wrote:

> (resent due to not adding mail-list into the CC)
> Hey there!
> Thanks for checking out the patch;
>
> As for videotoolbox_av1_start_frame:
> For the av_fast_realloc call it's mostly the same thing as
> ff_videotoolbox_buffer_copy defined at libavcodec/videotoolbox.c:79. It
> takes a pointer to a bitstream, its allocated size and updates it. Also
> there is a videotoolbox_common_decode_slice defined at libavcodec/
> videotoolbox:434, which does a pretty similar job to the one I mentioned,
> but a little different.
>
> As for videotoolbox_av1_decode_params:
> vtctx->bitstream at that moment shall be empty at that point, so does the
> vtctx->bitsream_size
> And as it turned out the sequence header obu needs to be copied into the
> bitstream for videotoolbox to decode a frame as mentioned in the thread
> on the apple forums(check the last but one comment)[1]
>
> So basically what's happening here:
> videotoolbox_av1_decode_params getting called the first in that decoding
> loop at libavcodec/av1dec.c:1243, it copies the sequence header obu into
> the bitstream as the first bytes(because at that point bitstream shall be
> zero), and increases the vtctx->bitstream_size by the size of the sequence
> header obu.
> Then videotoolbox_av1_start_frame is getting called at
> libavcodec/av1dec.c:1364 and copies the frame itself into the
> vtctx->bitstream, taking into account the shift needed by the
> vtctx->bitstream_size(which at that moment should be the size of the
> sequence header obu).
>
> [1] https://forums.developer.apple.com/forums/thread/739953
>
>
> On Sun, 12 May 2024 at 11:52, Ruslan Chernenko 
> wrote:
>
>> Hey there!
>> Thanks for checking out the patch;
>>
>> As for videotoolbox_av1_start_frame:
>> For the av_fast_realloc call it's mostly the same thing as
>> ff_videotoolbox_buffer_copy defined at libavcodec/videotoolbox.c:79. It
>> takes a pointer to a bitstream, its allocated size and updates it. Also
>> there is a videotoolbox_common_decode_slice defined at
>> libavcodec/videotoolbox:434, which does a pretty similar job to the one I
>> mentioned, but a little different.
>>
>> As for videotoolbox_av1_decode_params:
>> vtctx->bitstream at that moment shall be empty at that point, so does the
>> vtctx->bitsream_size
>> And as it turned out the sequence header obu needs to be copied into the
>> bitstream for videotoolbox to decode a frame as mentioned in the thread on
>> the apple forums(check the last but one comment)[1]
>>
>> So basically what's happening here:
>> videotoolbox_av1_decode_params getting called the first in that decoding
>> loop at libavcodec/av1dec.c:1243, it copies the sequence header obu into
>> the bitstream as the first bytes(because at that point bitstream shall be
>> zero), and increases the vtctx->bitstream_size by the size of the sequence
>> header obu.
>> Then videotoolbox_av1_start_frame is getting called at
>> libavcodec/av1dec.c:1364 and copies the frame itself into the
>> vtctx->bitstream, taking into account the shift needed by the
>> vtctx->bitstream_size(which at that moment should be the size of the
>> sequence header obu).
>>
>> [1] https://forums.developer.apple.com/forums/thread/739953
>>
>>
>> On Sun, 12 May 2024 at 04:52, Cameron Gutman 
>> wrote:
>>
>>> On Mon, May 6, 2024 at 3:45 PM Черненко Руслан 
>>> wrote:
>>> >
>>> > Signed-off-by: Chernenko Ruslan 
>>> > ---
>>> >   libavcodec/videotoolbox_av1.c | 102
>>> --
>>> >   1 file changed, 72 insertions(+), 30 deletions(-)
>>> >
>>> > diff --git a/libavcodec/videotoolbox_av1.c
>>> b/libavcodec/videotoolbox_av1.c
>>> > index 7f7270c466..736f2548db 100644
>>> > --- a/libavcodec/videotoolbox_av1.c
>>> > +++ b/libavcodec/videotoolbox_av1.c
>>> > @@ -19,50 +19,90 @@
>>> >* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> > 02110-1301 USA
>>> >*/
>>> >   -#include "libavformat/avio.h"
>>> > -#include "libavformat/avio_internal.h"
>>> > -#define CALLED_FROM_AVCODEC 1
>>> > -#include "libavformat/av1.c"
>>> > -#undef CALLED_FROM_AVCODEC
>>> >   -#include "libavutil/avutil.h"