[FFmpeg-devel,1/3] avcodec/cbs: add a helper to read extradata within packet side data

Using ff_cbs_read() on the raw buffer will not parse it as extradata,
resulting in parsing errors for example when handling ISOBMFF avcC.
This helper works around that.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/cbs.c | 13 +++++++++++++
 libavcodec/cbs.h |  4 ++++
 2 files changed, 17 insertions(+)

James Almer:
> Using ff_cbs_read() on the raw buffer will not parse it as extradata,
> resulting in parsing errors for example when handling ISOBMFF avcC.
> This helper works around that.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/cbs.c | 13 +++++++++++++
>  libavcodec/cbs.h |  4 ++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index 8d50ea1432..f6e371ddef 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -294,6 +294,19 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>                           pkt->data, pkt->size, 0);
>  }
>  
> +int ff_cbs_read_packet_side_data(CodedBitstreamContext *ctx,
> +                                 CodedBitstreamFragment *frag,
> +                                 const AVPacket *pkt)
> +{
> +    size_t side_data_size;
> +    const uint8_t *side_data =
> +        av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
> +                                &side_data_size);
> +
> +    return cbs_read_data(ctx, frag, NULL,
> +                         side_data, side_data_size, 1);
> +}
> +
>  int ff_cbs_read(CodedBitstreamContext *ctx,
>                  CodedBitstreamFragment *frag,
>                  const uint8_t *data, size_t size)
> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> index b7acf98347..bd97d163b1 100644
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -276,6 +276,10 @@ int ff_cbs_read_extradata_from_codec(CodedBitstreamContext *ctx,
>                                       CodedBitstreamFragment *frag,
>                                       const struct AVCodecContext *avctx);
>  
> +int ff_cbs_read_packet_side_data(CodedBitstreamContext *ctx,
> +                                 CodedBitstreamFragment *frag,
> +                                 const AVPacket *pkt);
> +
>  /**
>   * Read the data bitstream from a packet into a fragment, then
>   * split into units and decompose.
> 
Despite using this function in both 2 and 3 the callers still have to
check for whether extradata exists in the first place; in patch 3 this
is unavoidable. This makes me wonder whether it might not be better to
expose the header flag in ff_cbs_read().

- Andreas

On 8/5/2021 12:43 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Using ff_cbs_read() on the raw buffer will not parse it as extradata,
>> resulting in parsing errors for example when handling ISOBMFF avcC.
>> This helper works around that.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/cbs.c | 13 +++++++++++++
>>   libavcodec/cbs.h |  4 ++++
>>   2 files changed, 17 insertions(+)
>>
>> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
>> index 8d50ea1432..f6e371ddef 100644
>> --- a/libavcodec/cbs.c
>> +++ b/libavcodec/cbs.c
>> @@ -294,6 +294,19 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>>                            pkt->data, pkt->size, 0);
>>   }
>>   
>> +int ff_cbs_read_packet_side_data(CodedBitstreamContext *ctx,
>> +                                 CodedBitstreamFragment *frag,
>> +                                 const AVPacket *pkt)
>> +{
>> +    size_t side_data_size;
>> +    const uint8_t *side_data =
>> +        av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
>> +                                &side_data_size);
>> +
>> +    return cbs_read_data(ctx, frag, NULL,
>> +                         side_data, side_data_size, 1);
>> +}
>> +
>>   int ff_cbs_read(CodedBitstreamContext *ctx,
>>                   CodedBitstreamFragment *frag,
>>                   const uint8_t *data, size_t size)
>> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
>> index b7acf98347..bd97d163b1 100644
>> --- a/libavcodec/cbs.h
>> +++ b/libavcodec/cbs.h
>> @@ -276,6 +276,10 @@ int ff_cbs_read_extradata_from_codec(CodedBitstreamContext *ctx,
>>                                        CodedBitstreamFragment *frag,
>>                                        const struct AVCodecContext *avctx);
>>   
>> +int ff_cbs_read_packet_side_data(CodedBitstreamContext *ctx,
>> +                                 CodedBitstreamFragment *frag,
>> +                                 const AVPacket *pkt);
>> +
>>   /**
>>    * Read the data bitstream from a packet into a fragment, then
>>    * split into units and decompose.
>>
> Despite using this function in both 2 and 3 the callers still have to
> check for whether extradata exists in the first place; in patch 3 this
> is unavoidable. This makes me wonder whether it might not be better to
> expose the header flag in ff_cbs_read().
> 
> - Andreas

It's no different than existing callers of ff_cbs_read_extradata and 
ff_cbs_read_extradata_from_codec(), which check for avctx/par->extradata 
before calling either of them.
I personally prefer these helpers as is.

On 8/5/2021 12:24 PM, James Almer wrote:
> Using ff_cbs_read() on the raw buffer will not parse it as extradata,
> resulting in parsing errors for example when handling ISOBMFF avcC.
> This helper works around that.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>   libavcodec/cbs.c | 13 +++++++++++++
>   libavcodec/cbs.h |  4 ++++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c
> index 8d50ea1432..f6e371ddef 100644
> --- a/libavcodec/cbs.c
> +++ b/libavcodec/cbs.c
> @@ -294,6 +294,19 @@ int ff_cbs_read_packet(CodedBitstreamContext *ctx,
>                            pkt->data, pkt->size, 0);
>   }
>   
> +int ff_cbs_read_packet_side_data(CodedBitstreamContext *ctx,
> +                                 CodedBitstreamFragment *frag,
> +                                 const AVPacket *pkt)
> +{
> +    size_t side_data_size;
> +    const uint8_t *side_data =
> +        av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
> +                                &side_data_size);
> +
> +    return cbs_read_data(ctx, frag, NULL,
> +                         side_data, side_data_size, 1);
> +}
> +
>   int ff_cbs_read(CodedBitstreamContext *ctx,
>                   CodedBitstreamFragment *frag,
>                   const uint8_t *data, size_t size)
> diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h
> index b7acf98347..bd97d163b1 100644
> --- a/libavcodec/cbs.h
> +++ b/libavcodec/cbs.h
> @@ -276,6 +276,10 @@ int ff_cbs_read_extradata_from_codec(CodedBitstreamContext *ctx,
>                                        CodedBitstreamFragment *frag,
>                                        const struct AVCodecContext *avctx);
>   
> +int ff_cbs_read_packet_side_data(CodedBitstreamContext *ctx,
> +                                 CodedBitstreamFragment *frag,
> +                                 const AVPacket *pkt);
> +
>   /**
>    * Read the data bitstream from a packet into a fragment, then
>    * split into units and decompose.

Will apply set.