Re: [FFmpeg-devel] [PATCH 1/2] Revert "avcodec/decode: Fix avcodec parameters when bsfs, are enable by decoder"

[Zhao Zhili <quinkblack-at-foxmail.com@ffmpeg.org>]

> Otherwise the extradata used would be ISOBMFF if the input is
> even though we use the *_mp4toannexb BSFs to convert it to
> annex B to feed it to the actual decoder.
> 
> (The mediacodec decoders also use said BSFs, yet they process
> the extradata in a way that works even when using the ISOBMFF
> extradata; in fact, using the converted extradata would break
> their check for whether to warn for missing extradata for
> the ISOBMFF without-in-band-header profiles.
> 
> Furthermore, there are several users of the *_mp4toannexb BSFs
> that don't ever touch extradata. They have not been touched.)

The solution is fragile and easy to be broken. It doesn’t look like a better
solution than avcodec_parameters_to_context.

> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  libavcodec/amfdec.c     |  8 ++++++--
>  libavcodec/decode_bsf.h | 42 +++++++++++++++++++++++++++++++++++++++++
>  libavcodec/rkmppdec.c   |  9 ++++++---
>  3 files changed, 54 insertions(+), 5 deletions(-)
>  create mode 100644 libavcodec/decode_bsf.h
> 
> diff --git a/libavcodec/amfdec.c b/libavcodec/amfdec.c
> index 44ccd63aaf..1840a1b3c0 100644
> --- a/libavcodec/amfdec.c
> +++ b/libavcodec/amfdec.c
> @@ -25,6 +25,7 @@
>  #include "libavutil/mem.h"
>  #include "libavutil/time.h"
>  #include "decode.h"
> +#include "decode_bsf.h"
>  #include "libavutil/mastering_display_metadata.h"
>  
>  #if CONFIG_D3D11VA
> @@ -187,9 +188,12 @@ static int amf_init_decoder(AVCodecContext *avctx)
>          AMF_ASSIGN_PROPERTY_INT64(res, ctx->decoder, AMF_VIDEO_DECODER_SURFACE_COPY, ctx->copy_output);
>  
>      if (avctx->extradata_size) {
> -        res = amf_device_ctx->context->pVtbl->AllocBuffer(amf_device_ctx->context, AMF_MEMORY_HOST, avctx->extradata_size, &buffer);
> +        const uint8_t *extradata;
> +        int extradata_size;
> +        ff_decode_get_extradata(avctx, &extradata, &extradata_size);
> +        res = amf_device_ctx->context->pVtbl->AllocBuffer(amf_device_ctx->context, AMF_MEMORY_HOST, extradata_size, &buffer);
>          if (res == AMF_OK) {
> -            memcpy(buffer->pVtbl->GetNative(buffer), avctx->extradata, avctx->extradata_size);
> +            memcpy(buffer->pVtbl->GetNative(buffer), extradata, extradata_size);
>              AMF_ASSIGN_PROPERTY_INTERFACE(res,ctx->decoder, AMF_VIDEO_DECODER_EXTRADATA, buffer);
>              buffer->pVtbl->Release(buffer);
>              buffer = NULL;
> diff --git a/libavcodec/decode_bsf.h b/libavcodec/decode_bsf.h
> new file mode 100644
> index 0000000000..9ea9ab70c1
> --- /dev/null
> +++ b/libavcodec/decode_bsf.h
> @@ -0,0 +1,42 @@
> +/*
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#ifndef AVCODEC_DECODE_BSF_H
> +#define AVCODEC_DECODE_BSF_H
> +
> +#include <stdint.h>
> +
> +#include "avcodec.h"
> +#include "bsf.h"
> +#include "internal.h"
> +
> +/**
> + * Helper function for decoders that may use a BSF that changes extradata.
> + * This function will get the extradata from the BSF.
> + */
> +static inline void ff_decode_get_extradata(const AVCodecContext *avctx,
> +                                           const uint8_t **extradata,
> +                                           int *extradata_size)
> +{
> +    // Given that we unconditionally insert a null BSF when no BSF is
> +    // explicitly requested, we can just use the BSF's par_out here.
> +    *extradata      = avctx->internal->bsf->par_out->extradata;
> +    *extradata_size = avctx->internal->bsf->par_out->extradata_size;
> +}
> +
> +#endif /* AVCODEC_DECODE_BSF_H */
> diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
> index 54e697664e..dad0e4c25f 100644
> --- a/libavcodec/rkmppdec.c
> +++ b/libavcodec/rkmppdec.c
> @@ -29,6 +29,7 @@
>  #include "avcodec.h"
>  #include "codec_internal.h"
>  #include "decode.h"
> +#include "decode_bsf.h"
>  #include "hwconfig.h"
>  #include "libavutil/refstruct.h"
>  #include "libavutil/buffer.h"
> @@ -279,9 +280,11 @@ static int rkmpp_send_packet(AVCodecContext *avctx, const AVPacket *avpkt)
>      // on first packet, send extradata
>      if (decoder->first_packet) {
>          if (avctx->extradata_size) {
> -            ret = rkmpp_write_data(avctx, avctx->extradata,
> -                                            avctx->extradata_size,
> -                                            avpkt->pts);
> +            const uint8_t *extradata;
> +            int extradata_size;
> +            ff_decode_get_extradata(avctx, &extradata, &extradata_size);
> +            ret = rkmpp_write_data(avctx, (uint8_t*)extradata, extradata_size,
> +                                   avpkt->pts);
>              if (ret) {
>                  av_log(avctx, AV_LOG_ERROR, "Failed to write extradata to decoder (code = %d)\n", ret);
>                  return ret;
> -- 
> 2.45.2
> 

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