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

[Ruslan Chernenko <ractyfree@gmail.com>]

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 <ractyfree@gmail.com> 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 <ractyfree@gmail.com>
> 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 <aicommander@gmail.com>
>> wrote:
>>
>>> On Mon, May 6, 2024 at 3:45 PM Черненко Руслан <ractyfree@gmail.com>
>>> wrote:
>>> >
>>> > Signed-off-by: Chernenko Ruslan <ractyfree@gmail.com>
>>> > ---
>>> >   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) {
>>> > +
>>> > +    AV1DecContext *s = avctx->priv_data;
>>> > +    avctx->extradata = av_fast_realloc(avctx->extradata, +
>>> > &avctx->extradata_size, +            s->seq_data_ref->size + 4);
>>> > +    avctx->extradata_size = s->seq_data_ref->size + 4;
>>> > +    avctx->extradata[0] = 0x81; // version and marker (constant)
>>> > +    avctx->extradata[1] = s->raw_seq->seq_profile << 5 |
>>> > s->raw_seq->seq_level_idx[0];
>>> > +    avctx->extradata[2] = s->raw_seq->seq_tier[0] << 7 |
>>> > +                        s->raw_seq->color_config.high_bitdepth << 6 |
>>> +
>>> >                         s->raw_seq->color_config.twelve_bit << 5 |
>>> > +                        s->raw_seq->color_config.mono_chrome << 4 |
>>> > +                        s->raw_seq->color_config.subsampling_x << 3 |
>>> > +                        s->raw_seq->color_config.subsampling_y << 2 |
>>> > +
>>> s->raw_seq->color_config.chroma_sample_position;
>>> > +                                                                    +
>>> > +    if (s->raw_seq->initial_display_delay_present_flag) +
>>> > avctx->extradata[3] = 0 << 5 |
>>> > s->raw_seq->initial_display_delay_present_flag << 4 |
>>> > s->raw_seq->initial_display_delay_minus_1[0];
>>> > +    else
>>> > +        avctx->extradata[3] = 0x00;
>>> > +    memcpy(avctx->extradata + 4, s->seq_data_ref->data,
>>> > s->seq_data_ref->size);
>>> > +    CFDataRef data = CFDataCreate(kCFAllocatorDefault,
>>> > avctx->extradata, avctx->extradata_size);
>>> > +    return data;
>>> > +};
>>> > +
>>> > +
>>> > +static int videotoolbox_av1_decode_params(AVCodecContext *avctx,
>>> > +                                         int header_type,
>>> > +                                         const uint8_t *buffer,
>>> > +                                         uint32_t size)
>>> >   {
>>> > -    AVIOContext *pb;
>>> > -    uint8_t *buf;
>>> > -    CFDataRef data = NULL;
>>> > -    int buf_size = 0;
>>> > -    int ret = avio_open_dyn_buf(&pb);
>>> > -    if (ret < 0)
>>> > -        return NULL;
>>> > +    VTContext *vtctx = avctx->internal->hwaccel_priv_data;
>>> > +    void *tmp;
>>> >   -    ret = ff_isom_write_av1c(pb, avctx->extradata,
>>> > avctx->extradata_size, 1);
>>> > -    if (ret < 0)
>>> > -        goto fail;
>>> > +    tmp = av_fast_realloc(vtctx->bitstream,
>>> > +                         &vtctx->allocated_size,
>>> > +                         size);
>>>
>>> This should be 'size + vtctx->bitstream_size' right?
>>>
>>> You're going to write from vtctx->bitstream_size to
>>> (vtctx->bitstream_size + size).
>>>
>>> >   -    buf_size = avio_get_dyn_buf(pb, &buf);
>>> > -    if (buf_size)
>>> > -        data = CFDataCreate(kCFAllocatorDefault, buf, buf_size);
>>> > +    if (!tmp)
>>> > +        return AVERROR(ENOMEM);
>>> >   -fail:
>>> > -    ffio_free_dyn_buf(&pb);
>>> > +    vtctx->bitstream = tmp;
>>> >   -    return data;
>>> > +    // copy the sequence header obu into the bitstream
>>> > +    memcpy(vtctx->bitstream + vtctx->bitstream_size, +
>>> > buffer, +            size);
>>>
>>> It looks like your mail client mangled the patch here.
>>>
>>> > +
>>> > +    vtctx->bitstream_size += size;
>>> > +    return 0;
>>> >   }
>>> >    static int videotoolbox_av1_start_frame(AVCodecContext *avctx,
>>> >                                           const uint8_t *buffer,
>>> >                                           uint32_t size)
>>> >   {
>>> > +
>>> > +    +    VTContext *vtctx = avctx->internal->hwaccel_priv_data;
>>> > +    void *tmp;
>>> > +
>>> > +    tmp = av_fast_realloc(vtctx->bitstream,
>>> > +                         &vtctx->allocated_size,
>>> > +                         size);
>>> > +
>>>
>>> This should also be size + vtctx->bitstream_size.
>>>
>>> > +    if (!tmp)
>>> > +        return AVERROR(ENOMEM);
>>> > +
>>> > +    vtctx->bitstream = tmp;
>>> > +
>>> > +    // copy the frame data into the bitstream
>>> > +    memcpy(vtctx->bitstream + vtctx->bitstream_size, buffer, size);
>>> > +    vtctx->bitstream_size += size;
>>> >       return 0;
>>> >   }
>>> >   @@ -70,17 +110,18 @@ static int
>>> > videotoolbox_av1_decode_slice(AVCodecContext *avctx,
>>> >                                            const uint8_t *buffer,
>>> >                                            uint32_t size)
>>> >   {
>>> > -    VTContext *vtctx = avctx->internal->hwaccel_priv_data;
>>> > -
>>> > -    return ff_videotoolbox_buffer_copy(vtctx, buffer, size);
>>> > +    return 0;
>>> >   }
>>> >    static int videotoolbox_av1_end_frame(AVCodecContext *avctx)
>>> >   {
>>> >       const AV1DecContext *s = avctx->priv_data;
>>> > +    VTContext *vtctx = avctx->internal->hwaccel_priv_data;
>>> >       AVFrame *frame = s->cur_frame.f;
>>> >   -    return ff_videotoolbox_common_end_frame(avctx, frame);
>>> > +    int ret =  ff_videotoolbox_common_end_frame(avctx, frame);
>>> > +    vtctx->bitstream_size = 0;
>>> > +    return ret;
>>> >   }
>>> >    const FFHWAccel ff_av1_videotoolbox_hwaccel = {
>>> > @@ -89,6 +130,7 @@ const FFHWAccel ff_av1_videotoolbox_hwaccel = {
>>> >       .p.id           = AV_CODEC_ID_AV1,
>>> >       .p.pix_fmt      = AV_PIX_FMT_VIDEOTOOLBOX,
>>> >       .alloc_frame    = ff_videotoolbox_alloc_frame,
>>> > +    .decode_params  = videotoolbox_av1_decode_params,
>>> >       .start_frame    = videotoolbox_av1_start_frame,
>>> >       .decode_slice   = videotoolbox_av1_decode_slice,
>>> >       .end_frame      = videotoolbox_av1_end_frame,
>>> > --
>>> > 2.39.3 (Apple Git-146)
>>> >
>>> > _______________________________________________
>>> > 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".
>>>
>>
>>
>> --
>> C уважением,
>> Руслан.
>>
>> Mail: ractyfree@gmail.com
>> Telegram: @nenkoru
>> VK: https://vk.com/racty
>>
>
>
> --
> C уважением,
> Руслан.
>
> Mail: ractyfree@gmail.com
> Telegram: @nenkoru
> VK: https://vk.com/racty
>


-- 
C уважением,
Руслан.

Mail: ractyfree@gmail.com
Telegram: @nenkoru
VK: https://vk.com/racty
_______________________________________________
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".