[FFmpeg-devel] avcodec/mediacodecenc: Extract configOBUs from AV1CodecConfigurationRecord

From: Zhao Zhili <zhilizhao@tencent.com>

MediaCodec can generate AV1CodecConfigurationRecord, which shouldn't
be put into packet->data. Skip four bytes and extract configOBUs
if it exist.
---
I did some test on Pixel 8 Pro. AV1 hardware encoding works with a lot
of bugs:

1. It's broken for width non-aligned to 16. For width 1080 and pixel
format YUV420P, MediaCodec use 1080 as stride. For pixel format NV12,
MediaCodec use 1088 as stride. There is no API to get the stride info.
AMEDIAFORMAT_KEY_STRIDE doesn't work. And set stride to MediaCodec has
no effect from my test, at least on that device. We know the buffer
size provided by MediaCodec, but we still cannot get stride by
buf_size / height:

  1) For YUV420P, buf_size = 1080 * height
  2) For NV12, buf_size = 1080 + 1088 * (height - 1). Yes, buf_size doesn't
count last line's padding :(

2. After flush (which means reset for MediaCodec), it doesn't reset GOP
info, so it output a few non-key frames before generate a key frame.
This breaks what I did for AV_CODEC_FLAG_GLOBAL_HEADER: send a dummy
frame, send EOF, get packet and extract extra data, then reset
MediaCodec state by AMediaCodec_flush.

 libavcodec/mediacodecenc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

On 10/4/2024 2:54 PM, Zhao Zhili wrote:
> From: Zhao Zhili <zhilizhao@tencent.com>
> 
> MediaCodec can generate AV1CodecConfigurationRecord, which shouldn't
> be put into packet->data. Skip four bytes and extract configOBUs
> if it exist.

But why are you removing them from extradata if you only want to prevent 
them to make it to a packet?
You could instead make sure they are not dumped into the packet in the 
memcpy(pkt->data... below this chunk.

> ---
> I did some test on Pixel 8 Pro. AV1 hardware encoding works with a lot
> of bugs:
> 
> 1. It's broken for width non-aligned to 16. For width 1080 and pixel
> format YUV420P, MediaCodec use 1080 as stride. For pixel format NV12,
> MediaCodec use 1088 as stride. There is no API to get the stride info.
> AMEDIAFORMAT_KEY_STRIDE doesn't work. And set stride to MediaCodec has
> no effect from my test, at least on that device. We know the buffer
> size provided by MediaCodec, but we still cannot get stride by
> buf_size / height:
> 
>    1) For YUV420P, buf_size = 1080 * height
>    2) For NV12, buf_size = 1080 + 1088 * (height - 1). Yes, buf_size doesn't
> count last line's padding :(
> 
> 2. After flush (which means reset for MediaCodec), it doesn't reset GOP
> info, so it output a few non-key frames before generate a key frame.
> This breaks what I did for AV_CODEC_FLAG_GLOBAL_HEADER: send a dummy
> frame, send EOF, get packet and extract extra data, then reset
> MediaCodec state by AMediaCodec_flush.
> 
>   libavcodec/mediacodecenc.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
> index e76ea81236..3880ea2fe9 100644
> --- a/libavcodec/mediacodecenc.c
> +++ b/libavcodec/mediacodecenc.c
> @@ -429,6 +429,16 @@ static int mediacodec_receive(AVCodecContext *avctx, AVPacket *pkt)
>       }
>   
>       if (out_info.flags & ff_AMediaCodec_getBufferFlagCodecConfig(codec)) {
> +        if (avctx->codec_id == AV_CODEC_ID_AV1) {
> +            // Skip AV1CodecConfigurationRecord without configOBUs
> +            if (out_info.size <= 4) {
> +                ff_AMediaCodec_releaseOutputBuffer(codec, index, false);
> +                return mediacodec_receive(avctx, pkt);
> +            }
> +            out_info.size -= 4;
> +            out_info.offset += 4;
> +        }
> +
>           ret = av_reallocp(&s->extradata, out_info.size);
>           if (ret)
>               goto bailout;

On Sat, 5 Oct 2024, Zhao Zhili wrote:

> From: Zhao Zhili <zhilizhao@tencent.com>
>
> MediaCodec can generate AV1CodecConfigurationRecord, which shouldn't
> be put into packet->data. Skip four bytes and extract configOBUs
> if it exist.
> ---
> I did some test on Pixel 8 Pro. AV1 hardware encoding works with a lot
> of bugs:
>
> 1. It's broken for width non-aligned to 16. For width 1080 and pixel
> format YUV420P, MediaCodec use 1080 as stride. For pixel format NV12,
> MediaCodec use 1088 as stride. There is no API to get the stride info.
> AMEDIAFORMAT_KEY_STRIDE doesn't work. And set stride to MediaCodec has
> no effect from my test, at least on that device. We know the buffer
> size provided by MediaCodec, but we still cannot get stride by
> buf_size / height:
>
>  1) For YUV420P, buf_size = 1080 * height
>  2) For NV12, buf_size = 1080 + 1088 * (height - 1). Yes, buf_size doesn't
> count last line's padding :(

Isn't this pretty much the case for the encoders for other codecs as well 
- there aren't really any compat guarantees for how they behave for widths 
that aren't a multiple of 16? At least back when there when Android added 
CTS tests to guarantee some sort of cross device consistent behaviour, 
they only tested/mandated the behviour for a couple resolutions, that all 
were even multiples of 16.

I guess the difference here is whether it's possible to do cropping in the 
same way as via the h264_metadata/hevc_metadata BSFs?

// Martin

> On Oct 5, 2024, at 02:15, James Almer <jamrial@gmail.com> wrote:
> 
> On 10/4/2024 2:54 PM, Zhao Zhili wrote:
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> MediaCodec can generate AV1CodecConfigurationRecord, which shouldn't
>> be put into packet->data. Skip four bytes and extract configOBUs
>> if it exist.
> 
> But why are you removing them from extradata if you only want to prevent them to make it to a packet?
> You could instead make sure they are not dumped into the packet in the memcpy(pkt->data... below this chunk.

s->extradata is a temporary buffer, not avctx->extradata. I don’t know how to make
it useful with the whole AV1CodecConfigurationRecord. AV_CODEC_FLAG_GLOBAL_HEADER
is handled by extract_extradata bsf within MediaCodec wrapper, which is more reliable
than depends on OS to output extradata directly. From my test, the device output
AV1CodecConfigurationRecord with zero configOBUs.

> 
>> ---
>> I did some test on Pixel 8 Pro. AV1 hardware encoding works with a lot
>> of bugs:
>> 1. It's broken for width non-aligned to 16. For width 1080 and pixel
>> format YUV420P, MediaCodec use 1080 as stride. For pixel format NV12,
>> MediaCodec use 1088 as stride. There is no API to get the stride info.
>> AMEDIAFORMAT_KEY_STRIDE doesn't work. And set stride to MediaCodec has
>> no effect from my test, at least on that device. We know the buffer
>> size provided by MediaCodec, but we still cannot get stride by
>> buf_size / height:
>>   1) For YUV420P, buf_size = 1080 * height
>>   2) For NV12, buf_size = 1080 + 1088 * (height - 1). Yes, buf_size doesn't
>> count last line's padding :(
>> 2. After flush (which means reset for MediaCodec), it doesn't reset GOP
>> info, so it output a few non-key frames before generate a key frame.
>> This breaks what I did for AV_CODEC_FLAG_GLOBAL_HEADER: send a dummy
>> frame, send EOF, get packet and extract extra data, then reset
>> MediaCodec state by AMediaCodec_flush.
>>  libavcodec/mediacodecenc.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>> diff --git a/libavcodec/mediacodecenc.c b/libavcodec/mediacodecenc.c
>> index e76ea81236..3880ea2fe9 100644
>> --- a/libavcodec/mediacodecenc.c
>> +++ b/libavcodec/mediacodecenc.c
>> @@ -429,6 +429,16 @@ static int mediacodec_receive(AVCodecContext *avctx, AVPacket *pkt)
>>      }
>>        if (out_info.flags & ff_AMediaCodec_getBufferFlagCodecConfig(codec)) {
>> +        if (avctx->codec_id == AV_CODEC_ID_AV1) {
>> +            // Skip AV1CodecConfigurationRecord without configOBUs
>> +            if (out_info.size <= 4) {
>> +                ff_AMediaCodec_releaseOutputBuffer(codec, index, false);
>> +                return mediacodec_receive(avctx, pkt);
>> +            }
>> +            out_info.size -= 4;
>> +            out_info.offset += 4;
>> +        }
>> +
>>          ret = av_reallocp(&s->extradata, out_info.size);
>>          if (ret)
>>              goto bailout;
> 
> _______________________________________________
> 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".
>

> On Oct 5, 2024, at 05:01, Martin Storsjö <martin@martin.st> wrote:
> 
> On Sat, 5 Oct 2024, Zhao Zhili wrote:
> 
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> MediaCodec can generate AV1CodecConfigurationRecord, which shouldn't
>> be put into packet->data. Skip four bytes and extract configOBUs
>> if it exist.
>> ---
>> I did some test on Pixel 8 Pro. AV1 hardware encoding works with a lot
>> of bugs:
>> 
>> 1. It's broken for width non-aligned to 16. For width 1080 and pixel
>> format YUV420P, MediaCodec use 1080 as stride. For pixel format NV12,
>> MediaCodec use 1088 as stride. There is no API to get the stride info.
>> AMEDIAFORMAT_KEY_STRIDE doesn't work. And set stride to MediaCodec has
>> no effect from my test, at least on that device. We know the buffer
>> size provided by MediaCodec, but we still cannot get stride by
>> buf_size / height:
>> 
>> 1) For YUV420P, buf_size = 1080 * height
>> 2) For NV12, buf_size = 1080 + 1088 * (height - 1). Yes, buf_size doesn't
>> count last line's padding :(
> 
> Isn't this pretty much the case for the encoders for other codecs as well - there aren't really any compat guarantees for how they behave for widths that aren't a multiple of 16? At least back when there when Android added CTS tests to guarantee some sort of cross device consistent behaviour, they only tested/mandated the behviour for a couple resolutions, that all were even multiples of 16.

> 
> I guess the difference here is whether it's possible to do cropping in the same way as via the h264_metadata/hevc_metadata BSFs?

All codecs have the same issue for dst buffer stride. Yes we can workaround it for H.264/H.265 with
h264_metadata/hevc_metadata BSFs. I think we should do the same for AV1.

Even without dst buffer stride issue, some devices still have align requirement on video size. For
example, when send frame to encoder via Surface/ANativeWindow, there is no explicit memcpy and
OS handles alignment of the underlying buffer. A 1080x1920 video turn into 1072x1920 after encoding
with MediaTek device (not with FFmpeg MediaCodec wrapper but call MediaCodec API directly in App).
Take care of alignment/crop within our wrapper rather than leave it to OS implementation resolved these
mess.

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