[FFmpeg-devel] [RFC PATCH 4/4] avcodec/h265_metadata_bsf: add nuh_layer_id option

[Mark Thompson]

On 05/01/2023 16:14, Zhao Zhili wrote:
> From: Zhao Zhili <zhilizhao at tencent.com>
> 
> Extract NALUs with the specified nuh_layer_id and rewrite as base
> layer. For example, to extract alpha layer with nuh_layer_id equal
> to 1:
> 
> ./ffmpeg -i alpha.mp4 \
> 	-an -c:v copy \
> 	-bsf:v hevc_mp4toannexb,hevc_metadata=nuh_layer_id=1 \
> 	alpha.hevc
> ---
>   doc/bitstream_filters.texi     |  5 +++++
>   libavcodec/h265_metadata_bsf.c | 25 +++++++++++++++++++++++++
>   libavcodec/version.h           |  2 +-
>   3 files changed, 31 insertions(+), 1 deletion(-)

Rest of series looks sensible to me.

This doesn't have anything to do with metadata, though?  IMO make a new BSF (extractlayer?) for this; the amount of boilerplate is quite small.

> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
> index c63c20370f..8cb5bee644 100644
> --- a/doc/bitstream_filters.texi
> +++ b/doc/bitstream_filters.texi
> @@ -443,6 +443,11 @@ The argument must be the name of a level (for example, @samp{5.1}), a
>   or the special name @samp{auto} indicating that the filter should
>   attempt to guess the level from the input stream properties.
>   
> + at item nuh_layer_id
> +
> +Extract NALUs with the specified nuh_layer_id and rewrite as base layer.
> +All other NALUs are dropped except VPS.
> +
>   @end table
>   
>   @section hevc_mp4toannexb
> diff --git a/libavcodec/h265_metadata_bsf.c b/libavcodec/h265_metadata_bsf.c
> index 6787bd14a1..fdaab27186 100644
> --- a/libavcodec/h265_metadata_bsf.c
> +++ b/libavcodec/h265_metadata_bsf.c
> @@ -62,6 +62,8 @@ typedef struct H265MetadataContext {
>       int level;
>       int level_guess;
>       int level_warned;
> +
> +    int nuh_layer_id;
>   } H265MetadataContext;
>   
>   
> @@ -328,6 +330,25 @@ static int h265_metadata_update_fragment(AVBSFContext *bsf, AVPacket *pkt,
>       H265MetadataContext *ctx = bsf->priv_data;
>       int err, i;
>   
> +    if (ctx->nuh_layer_id >= 0) {
> +        H265RawNALUnitHeader *header;
> +        for (i = 0; i < au->nb_units; i++) {
> +            if (au->units[i].type == HEVC_NAL_VPS) {
> +                H265RawVPS *vps = au->units[i].content;
> +                vps->vps_extension_flag = 0;
> +                continue;
> +            }

What is this change doing?

Does the VPS also need to be edited to fix the layer references?  (The intended input presumably has a VPS saying there are two layers which are both included, but after this change there aren't.)

> +
> +            header = au->units[i].content;
> +            if (header->nuh_layer_id != ctx->nuh_layer_id) {
> +                ff_cbs_delete_unit(au, i);
> +                i--;
> +                continue;
> +            }
> +            header->nuh_layer_id = 0;
> +        }
> +    }

Not required, but some error messages for bad cases would be nice - you can know from the VPS if the asked-for layer doesn't exist at all and stop early, and you can know later that there was no output and warn the user about it (bonus points if you also tell them which layers are present!).

> +
>       // If an AUD is present, it must be the first NAL unit.
>       if (au->nb_units && au->units[0].type == HEVC_NAL_AUD) {
>           if (ctx->aud == BSF_ELEMENT_REMOVE)
> @@ -478,6 +499,10 @@ static const AVOption h265_metadata_options[] = {
>       { LEVEL("8.5", 255) },
>   #undef LEVEL
>   
> +    { "nuh_layer_id", "Extract NALUs with the specified nuh_layer_id and rewrite as base layer",
> +        OFFSET(nuh_layer_id), AV_OPT_TYPE_INT,
> +        { .i64 = -1 }, -1, 62, FLAGS },
> +
>       { NULL }
>   };

Thanks,

- Mark