diff mbox series

[FFmpeg-devel] Add advance vps/sps/pps id to hevc_metadata bsf

Message ID 20200108131019.23930-1-eran.gonen@cloudinary.com
State New
Headers show
Series [FFmpeg-devel] Add advance vps/sps/pps id to hevc_metadata bsf
Related show

Checks

Context Check Description
andriy/ffmpeg-patchwork pending
andriy/ffmpeg-patchwork success Applied patch
andriy/ffmpeg-patchwork success Configure finished
andriy/ffmpeg-patchwork success Make finished
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Eran Gonen Jan. 8, 2020, 1:10 p.m. UTC
---
 libavcodec/h265_metadata_bsf.c | 117 +++++++++++++++++++++++++++++++--
 1 file changed, 111 insertions(+), 6 deletions(-)

Comments

Andreas Rheinhardt Jan. 8, 2020, 1:31 p.m. UTC | #1
On Wed, Jan 8, 2020 at 2:18 PM Eran Gonen <eran.gonen@cloudinary.com> wrote:

> ---
>  libavcodec/h265_metadata_bsf.c | 117 +++++++++++++++++++++++++++++++--
>  1 file changed, 111 insertions(+), 6 deletions(-)
>
> diff --git a/libavcodec/h265_metadata_bsf.c
> b/libavcodec/h265_metadata_bsf.c
> index b3a1fda144..796046ac7f 100644
> --- a/libavcodec/h265_metadata_bsf.c
> +++ b/libavcodec/h265_metadata_bsf.c
> @@ -68,6 +68,10 @@ typedef struct H265MetadataContext {
>      int level;
>      int level_guess;
>      int level_warned;
> +
> +    int advance_vps_id;
> +    int advance_sps_id;
> +    int advance_pps_id;
>  } H265MetadataContext;
>
>
> @@ -168,6 +172,10 @@ static int h265_metadata_update_vps(AVBSFContext *bsf,
>  {
>      H265MetadataContext *ctx = bsf->priv_data;
>
> +    if(ctx->advance_vps_id > -1) {
> +        vps->vps_video_parameter_set_id =
> (vps->vps_video_parameter_set_id + ctx->advance_vps_id) %
> HEVC_MAX_VPS_COUNT;
> +    }
> +
>      if (ctx->tick_rate.num && ctx->tick_rate.den) {
>          int num, den;
>
> @@ -200,6 +208,13 @@ static int h265_metadata_update_sps(AVBSFContext *bsf,
>      int need_vui = 0;
>      int crop_unit_x, crop_unit_y;
>
> +    if(ctx->advance_vps_id > -1) {
> +        sps->sps_video_parameter_set_id =
> (sps->sps_video_parameter_set_id + ctx->advance_vps_id) %
> HEVC_MAX_VPS_COUNT;
> +    }
> +    if(ctx->advance_sps_id > -1) {
> +        sps->sps_seq_parameter_set_id = (sps->sps_seq_parameter_set_id +
> ctx->advance_sps_id) % HEVC_MAX_SPS_COUNT;
> +    }
> +
>      if (ctx->sample_aspect_ratio.num && ctx->sample_aspect_ratio.den) {
>          // Table E-1.
>          static const AVRational sar_idc[] = {
> @@ -336,6 +351,32 @@ static int h265_metadata_update_sps(AVBSFContext *bsf,
>      return 0;
>  }
>
> +static int h265_metadata_update_pps(AVBSFContext *bsf,
> +                                    H265RawPPS *pps)
> +{
> +    H265MetadataContext *ctx = bsf->priv_data;
> +
> +    if(ctx->advance_sps_id > -1) {
> +        pps->pps_seq_parameter_set_id = (pps->pps_seq_parameter_set_id +
> ctx->advance_sps_id) % HEVC_MAX_SPS_COUNT;
> +    }
> +    if(ctx->advance_pps_id > -1) {
> +        pps->pps_pic_parameter_set_id = (pps->pps_pic_parameter_set_id +
> ctx->advance_pps_id) % HEVC_MAX_PPS_COUNT;
> +    }
> +
> +    return 0;
> +}
> +
> +static int h265_metadata_update_slice_header(AVBSFContext *bsf,
> +        H265RawSliceHeader *slice)
> +{
> +    H265MetadataContext *ctx = bsf->priv_data;
> +
> +    if(ctx->advance_pps_id > -1) {
> +        slice->slice_pic_parameter_set_id =
> (slice->slice_pic_parameter_set_id + ctx->advance_pps_id) %
> HEVC_MAX_PPS_COUNT;
> +    }
> +    return 0;
> +}
> +
>  static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
>  {
>      H265MetadataContext *ctx = bsf->priv_data;
> @@ -406,15 +447,42 @@ static int h265_metadata_filter(AVBSFContext *bsf,
> AVPacket *pkt)
>          h265_metadata_guess_level(bsf, au);
>
>      for (i = 0; i < au->nb_units; i++) {
> -        if (au->units[i].type == HEVC_NAL_VPS) {
> +        switch (au->units[i].type) {
> +        case HEVC_NAL_VPS:
>              err = h265_metadata_update_vps(bsf, au->units[i].content);
>              if (err < 0)
>                  goto fail;
> -        }
> -        if (au->units[i].type == HEVC_NAL_SPS) {
> +            break;
> +        case HEVC_NAL_SPS:
>              err = h265_metadata_update_sps(bsf, au->units[i].content);
>              if (err < 0)
>                  goto fail;
> +            break;
> +        case HEVC_NAL_PPS:
> +            err = h265_metadata_update_pps(bsf, au->units[i].content);
> +            if (err < 0)
> +                goto fail;
> +            break;
> +        case HEVC_NAL_TRAIL_N:
> +        case HEVC_NAL_TRAIL_R:
> +        case HEVC_NAL_TSA_N:
> +        case HEVC_NAL_TSA_R:
> +        case HEVC_NAL_STSA_N:
> +        case HEVC_NAL_STSA_R:
> +        case HEVC_NAL_BLA_W_LP:
> +        case HEVC_NAL_BLA_W_RADL:
> +        case HEVC_NAL_BLA_N_LP:
> +        case HEVC_NAL_IDR_W_RADL:
> +        case HEVC_NAL_IDR_N_LP:
> +        case HEVC_NAL_CRA_NUT:
> +        case HEVC_NAL_RADL_N:
> +        case HEVC_NAL_RADL_R:
> +        case HEVC_NAL_RASL_N:
> +        case HEVC_NAL_RASL_R:
> +            err = h265_metadata_update_slice_header(bsf,
> au->units[i].content);
> +            if (err < 0)
> +                goto fail;
> +            break;
>          }
>      }
>
> @@ -455,15 +523,42 @@ static int h265_metadata_init(AVBSFContext *bsf)
>              h265_metadata_guess_level(bsf, au);
>
>          for (i = 0; i < au->nb_units; i++) {
> -            if (au->units[i].type == HEVC_NAL_VPS) {
> +            switch (au->units[i].type) {
> +            case HEVC_NAL_VPS:
>                  err = h265_metadata_update_vps(bsf, au->units[i].content);
>                  if (err < 0)
>                      goto fail;
> -            }
> -            if (au->units[i].type == HEVC_NAL_SPS) {
> +                break;
> +            case HEVC_NAL_SPS:
>                  err = h265_metadata_update_sps(bsf, au->units[i].content);
>                  if (err < 0)
>                      goto fail;
> +                break;
> +            case HEVC_NAL_PPS:
> +                err = h265_metadata_update_pps(bsf, au->units[i].content);
> +                if (err < 0)
> +                    goto fail;
> +                break;
> +            case HEVC_NAL_TRAIL_N:
> +            case HEVC_NAL_TRAIL_R:
> +            case HEVC_NAL_TSA_N:
> +            case HEVC_NAL_TSA_R:
> +            case HEVC_NAL_STSA_N:
> +            case HEVC_NAL_STSA_R:
> +            case HEVC_NAL_BLA_W_LP:
> +            case HEVC_NAL_BLA_W_RADL:
> +            case HEVC_NAL_BLA_N_LP:
> +            case HEVC_NAL_IDR_W_RADL:
> +            case HEVC_NAL_IDR_N_LP:
> +            case HEVC_NAL_CRA_NUT:
> +            case HEVC_NAL_RADL_N:
> +            case HEVC_NAL_RADL_R:
> +            case HEVC_NAL_RASL_N:
> +            case HEVC_NAL_RASL_R:
> +                err = h265_metadata_update_slice_header(bsf,
> au->units[i].content);
> +                if (err < 0)
> +                    goto fail;
> +                break;
>              }
>          }
>
> @@ -547,6 +642,16 @@ static const AVOption h265_metadata_options[] = {
>          OFFSET(crop_bottom), AV_OPT_TYPE_INT,
>          { .i64 = -1 }, -1, HEVC_MAX_HEIGHT, FLAGS },
>
> +    { "advance_vps_id", "Advance the vps id",
> +        OFFSET(advance_vps_id), AV_OPT_TYPE_INT,
> +        { .i64 = -1 }, -1, HEVC_MAX_VPS_COUNT, FLAGS },
> +    {  "advance_sps_id", "Advance the sps id",
> +        OFFSET(advance_sps_id), AV_OPT_TYPE_INT,
> +        { .i64 = -1 }, -1, HEVC_MAX_SPS_COUNT, FLAGS },
> +    {  "advance_pps_id", "Advance the pps id",
> +        OFFSET(advance_pps_id), AV_OPT_TYPE_INT,
> +        { .i64 = -1 }, -1, HEVC_MAX_PPS_COUNT, FLAGS },
> +
>

I wonder whether it would not be simpler to set the default values of these
options to zero and apply the offsets unconditionally.

- Andreas
Hendrik Leppkes Jan. 8, 2020, 1:58 p.m. UTC | #2
On Wed, Jan 8, 2020 at 2:18 PM Eran Gonen <eran.gonen@cloudinary.com> wrote:
> @@ -547,6 +642,16 @@ static const AVOption h265_metadata_options[] = {
>          OFFSET(crop_bottom), AV_OPT_TYPE_INT,
>          { .i64 = -1 }, -1, HEVC_MAX_HEIGHT, FLAGS },
>
> +    { "advance_vps_id", "Advance the vps id",
> +        OFFSET(advance_vps_id), AV_OPT_TYPE_INT,
> +        { .i64 = -1 }, -1, HEVC_MAX_VPS_COUNT, FLAGS },
> +    {  "advance_sps_id", "Advance the sps id",
> +        OFFSET(advance_sps_id), AV_OPT_TYPE_INT,
> +        { .i64 = -1 }, -1, HEVC_MAX_SPS_COUNT, FLAGS },
> +    {  "advance_pps_id", "Advance the pps id",
> +        OFFSET(advance_pps_id), AV_OPT_TYPE_INT,
> +        { .i64 = -1 }, -1, HEVC_MAX_PPS_COUNT, FLAGS },
> +

I think these could use some better wording and/or documentation, I'm
not sure many people would understand whats going to happen here.

- Hendrik
Eran Gonen Jan. 8, 2020, 2:03 p.m. UTC | #3
It would, I'll change it.

Thanks,
Eran

On Wed, Jan 8, 2020 at 3:32 PM Andreas Rheinhardt <
andreas.rheinhardt@gmail.com> wrote:

> On Wed, Jan 8, 2020 at 2:18 PM Eran Gonen <eran.gonen@cloudinary.com>
> wrote:
>
> > ---
> >  libavcodec/h265_metadata_bsf.c | 117 +++++++++++++++++++++++++++++++--
> >  1 file changed, 111 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavcodec/h265_metadata_bsf.c
> > b/libavcodec/h265_metadata_bsf.c
> > index b3a1fda144..796046ac7f 100644
> > --- a/libavcodec/h265_metadata_bsf.c
> > +++ b/libavcodec/h265_metadata_bsf.c
> > @@ -68,6 +68,10 @@ typedef struct H265MetadataContext {
> >      int level;
> >      int level_guess;
> >      int level_warned;
> > +
> > +    int advance_vps_id;
> > +    int advance_sps_id;
> > +    int advance_pps_id;
> >  } H265MetadataContext;
> >
> >
> > @@ -168,6 +172,10 @@ static int h265_metadata_update_vps(AVBSFContext
> *bsf,
> >  {
> >      H265MetadataContext *ctx = bsf->priv_data;
> >
> > +    if(ctx->advance_vps_id > -1) {
> > +        vps->vps_video_parameter_set_id =
> > (vps->vps_video_parameter_set_id + ctx->advance_vps_id) %
> > HEVC_MAX_VPS_COUNT;
> > +    }
> > +
> >      if (ctx->tick_rate.num && ctx->tick_rate.den) {
> >          int num, den;
> >
> > @@ -200,6 +208,13 @@ static int h265_metadata_update_sps(AVBSFContext
> *bsf,
> >      int need_vui = 0;
> >      int crop_unit_x, crop_unit_y;
> >
> > +    if(ctx->advance_vps_id > -1) {
> > +        sps->sps_video_parameter_set_id =
> > (sps->sps_video_parameter_set_id + ctx->advance_vps_id) %
> > HEVC_MAX_VPS_COUNT;
> > +    }
> > +    if(ctx->advance_sps_id > -1) {
> > +        sps->sps_seq_parameter_set_id = (sps->sps_seq_parameter_set_id +
> > ctx->advance_sps_id) % HEVC_MAX_SPS_COUNT;
> > +    }
> > +
> >      if (ctx->sample_aspect_ratio.num && ctx->sample_aspect_ratio.den) {
> >          // Table E-1.
> >          static const AVRational sar_idc[] = {
> > @@ -336,6 +351,32 @@ static int h265_metadata_update_sps(AVBSFContext
> *bsf,
> >      return 0;
> >  }
> >
> > +static int h265_metadata_update_pps(AVBSFContext *bsf,
> > +                                    H265RawPPS *pps)
> > +{
> > +    H265MetadataContext *ctx = bsf->priv_data;
> > +
> > +    if(ctx->advance_sps_id > -1) {
> > +        pps->pps_seq_parameter_set_id = (pps->pps_seq_parameter_set_id +
> > ctx->advance_sps_id) % HEVC_MAX_SPS_COUNT;
> > +    }
> > +    if(ctx->advance_pps_id > -1) {
> > +        pps->pps_pic_parameter_set_id = (pps->pps_pic_parameter_set_id +
> > ctx->advance_pps_id) % HEVC_MAX_PPS_COUNT;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +static int h265_metadata_update_slice_header(AVBSFContext *bsf,
> > +        H265RawSliceHeader *slice)
> > +{
> > +    H265MetadataContext *ctx = bsf->priv_data;
> > +
> > +    if(ctx->advance_pps_id > -1) {
> > +        slice->slice_pic_parameter_set_id =
> > (slice->slice_pic_parameter_set_id + ctx->advance_pps_id) %
> > HEVC_MAX_PPS_COUNT;
> > +    }
> > +    return 0;
> > +}
> > +
> >  static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
> >  {
> >      H265MetadataContext *ctx = bsf->priv_data;
> > @@ -406,15 +447,42 @@ static int h265_metadata_filter(AVBSFContext *bsf,
> > AVPacket *pkt)
> >          h265_metadata_guess_level(bsf, au);
> >
> >      for (i = 0; i < au->nb_units; i++) {
> > -        if (au->units[i].type == HEVC_NAL_VPS) {
> > +        switch (au->units[i].type) {
> > +        case HEVC_NAL_VPS:
> >              err = h265_metadata_update_vps(bsf, au->units[i].content);
> >              if (err < 0)
> >                  goto fail;
> > -        }
> > -        if (au->units[i].type == HEVC_NAL_SPS) {
> > +            break;
> > +        case HEVC_NAL_SPS:
> >              err = h265_metadata_update_sps(bsf, au->units[i].content);
> >              if (err < 0)
> >                  goto fail;
> > +            break;
> > +        case HEVC_NAL_PPS:
> > +            err = h265_metadata_update_pps(bsf, au->units[i].content);
> > +            if (err < 0)
> > +                goto fail;
> > +            break;
> > +        case HEVC_NAL_TRAIL_N:
> > +        case HEVC_NAL_TRAIL_R:
> > +        case HEVC_NAL_TSA_N:
> > +        case HEVC_NAL_TSA_R:
> > +        case HEVC_NAL_STSA_N:
> > +        case HEVC_NAL_STSA_R:
> > +        case HEVC_NAL_BLA_W_LP:
> > +        case HEVC_NAL_BLA_W_RADL:
> > +        case HEVC_NAL_BLA_N_LP:
> > +        case HEVC_NAL_IDR_W_RADL:
> > +        case HEVC_NAL_IDR_N_LP:
> > +        case HEVC_NAL_CRA_NUT:
> > +        case HEVC_NAL_RADL_N:
> > +        case HEVC_NAL_RADL_R:
> > +        case HEVC_NAL_RASL_N:
> > +        case HEVC_NAL_RASL_R:
> > +            err = h265_metadata_update_slice_header(bsf,
> > au->units[i].content);
> > +            if (err < 0)
> > +                goto fail;
> > +            break;
> >          }
> >      }
> >
> > @@ -455,15 +523,42 @@ static int h265_metadata_init(AVBSFContext *bsf)
> >              h265_metadata_guess_level(bsf, au);
> >
> >          for (i = 0; i < au->nb_units; i++) {
> > -            if (au->units[i].type == HEVC_NAL_VPS) {
> > +            switch (au->units[i].type) {
> > +            case HEVC_NAL_VPS:
> >                  err = h265_metadata_update_vps(bsf,
> au->units[i].content);
> >                  if (err < 0)
> >                      goto fail;
> > -            }
> > -            if (au->units[i].type == HEVC_NAL_SPS) {
> > +                break;
> > +            case HEVC_NAL_SPS:
> >                  err = h265_metadata_update_sps(bsf,
> au->units[i].content);
> >                  if (err < 0)
> >                      goto fail;
> > +                break;
> > +            case HEVC_NAL_PPS:
> > +                err = h265_metadata_update_pps(bsf,
> au->units[i].content);
> > +                if (err < 0)
> > +                    goto fail;
> > +                break;
> > +            case HEVC_NAL_TRAIL_N:
> > +            case HEVC_NAL_TRAIL_R:
> > +            case HEVC_NAL_TSA_N:
> > +            case HEVC_NAL_TSA_R:
> > +            case HEVC_NAL_STSA_N:
> > +            case HEVC_NAL_STSA_R:
> > +            case HEVC_NAL_BLA_W_LP:
> > +            case HEVC_NAL_BLA_W_RADL:
> > +            case HEVC_NAL_BLA_N_LP:
> > +            case HEVC_NAL_IDR_W_RADL:
> > +            case HEVC_NAL_IDR_N_LP:
> > +            case HEVC_NAL_CRA_NUT:
> > +            case HEVC_NAL_RADL_N:
> > +            case HEVC_NAL_RADL_R:
> > +            case HEVC_NAL_RASL_N:
> > +            case HEVC_NAL_RASL_R:
> > +                err = h265_metadata_update_slice_header(bsf,
> > au->units[i].content);
> > +                if (err < 0)
> > +                    goto fail;
> > +                break;
> >              }
> >          }
> >
> > @@ -547,6 +642,16 @@ static const AVOption h265_metadata_options[] = {
> >          OFFSET(crop_bottom), AV_OPT_TYPE_INT,
> >          { .i64 = -1 }, -1, HEVC_MAX_HEIGHT, FLAGS },
> >
> > +    { "advance_vps_id", "Advance the vps id",
> > +        OFFSET(advance_vps_id), AV_OPT_TYPE_INT,
> > +        { .i64 = -1 }, -1, HEVC_MAX_VPS_COUNT, FLAGS },
> > +    {  "advance_sps_id", "Advance the sps id",
> > +        OFFSET(advance_sps_id), AV_OPT_TYPE_INT,
> > +        { .i64 = -1 }, -1, HEVC_MAX_SPS_COUNT, FLAGS },
> > +    {  "advance_pps_id", "Advance the pps id",
> > +        OFFSET(advance_pps_id), AV_OPT_TYPE_INT,
> > +        { .i64 = -1 }, -1, HEVC_MAX_PPS_COUNT, FLAGS },
> > +
> >
>
> I wonder whether it would not be simpler to set the default values of these
> options to zero and apply the offsets unconditionally.
>
> - Andreas
> _______________________________________________
> 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".
Eran Gonen Jan. 8, 2020, 2:10 p.m. UTC | #4
Advance pps id means advance pps id. What would you suggest?

On Wed, Jan 8, 2020 at 4:04 PM Hendrik Leppkes <h.leppkes@gmail.com> wrote:

> On Wed, Jan 8, 2020 at 2:18 PM Eran Gonen <eran.gonen@cloudinary.com>
> wrote:
> > @@ -547,6 +642,16 @@ static const AVOption h265_metadata_options[] = {
> >          OFFSET(crop_bottom), AV_OPT_TYPE_INT,
> >          { .i64 = -1 }, -1, HEVC_MAX_HEIGHT, FLAGS },
> >
> > +    { "advance_vps_id", "Advance the vps id",
> > +        OFFSET(advance_vps_id), AV_OPT_TYPE_INT,
> > +        { .i64 = -1 }, -1, HEVC_MAX_VPS_COUNT, FLAGS },
> > +    {  "advance_sps_id", "Advance the sps id",
> > +        OFFSET(advance_sps_id), AV_OPT_TYPE_INT,
> > +        { .i64 = -1 }, -1, HEVC_MAX_SPS_COUNT, FLAGS },
> > +    {  "advance_pps_id", "Advance the pps id",
> > +        OFFSET(advance_pps_id), AV_OPT_TYPE_INT,
> > +        { .i64 = -1 }, -1, HEVC_MAX_PPS_COUNT, FLAGS },
> > +
>
> I think these could use some better wording and/or documentation, I'm
> not sure many people would understand whats going to happen here.
>
> - Hendrik
> _______________________________________________
> 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".
Moritz Barsnick Jan. 8, 2020, 2:18 p.m. UTC | #5
On Wed, Jan 08, 2020 at 16:10:12 +0200, Eran Gonen wrote:
> Advance pps id means advance pps id. What would you suggest?

You mean, mathematically speaking, "increase"?

You didn't fix the "if(" -> "if (" style isse.

And as Hendrik mentioned (I missed that originally): The
"hevc_metadata" section in doc/bitstream_filters.texi needs to be
amended.

Cheers,
Moritz
Eran Gonen Jan. 8, 2020, 2:25 p.m. UTC | #6
I will fix the style and documentation and resubmit.
{ "increase_vps_id", "Increase the vps id",
Is this naming more clear to you?

On Wed, Jan 8, 2020 at 4:18 PM Moritz Barsnick <barsnick@gmx.net> wrote:

> On Wed, Jan 08, 2020 at 16:10:12 +0200, Eran Gonen wrote:
> > Advance pps id means advance pps id. What would you suggest?
>
> You mean, mathematically speaking, "increase"?
>
> You didn't fix the "if(" -> "if (" style isse.
>
> And as Hendrik mentioned (I missed that originally): The
> "hevc_metadata" section in doc/bitstream_filters.texi needs to be
> amended.
>
> Cheers,
> Moritz
> _______________________________________________
> 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".
Anton Khirnov Jan. 8, 2020, 2:42 p.m. UTC | #7
Quoting Eran Gonen (2020-01-08 15:25:20)
> I will fix the style and documentation and resubmit.
> { "increase_vps_id", "Increase the vps id",
> Is this naming more clear to you?

If negative values are allowed, then 'offset' would be more accurate.
Eran Gonen Jan. 8, 2020, 2:44 p.m. UTC | #8
It's limited to [0 - 16] (positive)

On Wed, Jan 8, 2020 at 4:42 PM Anton Khirnov <anton@khirnov.net> wrote:

> Quoting Eran Gonen (2020-01-08 15:25:20)
> > I will fix the style and documentation and resubmit.
> > { "increase_vps_id", "Increase the vps id",
> > Is this naming more clear to you?
>
> If negative values are allowed, then 'offset' would be more accurate.
>
> --
> Anton Khirnov
> _______________________________________________
> 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".
Moritz Barsnick Jan. 8, 2020, 2:44 p.m. UTC | #9
On Wed, Jan 08, 2020 at 16:25:20 +0200, Eran Gonen wrote:
> { "increase_vps_id", "Increase the vps id",
> Is this naming more clear to you?

If that's what it does, then this is indeed clearer to me.

But since it's not boolean, it's a number, it should perhaps reflect
that. I can't come up with something good, but perhaps: "amount to
increase the VPS ID by" or something like this.

Sorry for nitpicking,Moritz
Moritz Barsnick Jan. 8, 2020, 2:46 p.m. UTC | #10
On Wed, Jan 08, 2020 at 15:42:09 +0100, Anton Khirnov wrote:
> If negative values are allowed, then 'offset' would be more accurate.

That's the word I was looking for, nice. Even valid if only positive
offsets are allowed. (I understand you implemented wrap-around. Perhaps
sufficient to write that in the documentation. Or implement both
directions. ;-))

Moritz
Eran Gonen Jan. 8, 2020, 3:09 p.m. UTC | #11
Thanks for your feedback.

@item vps_id_offset
@item sps_id_offset
@item pps_id_offset
Replace the original VPS/SPS/PPS identifier by adding an offset.

Offset sounds just as good to me. Is this clear?

On Wed, Jan 8, 2020 at 4:46 PM Moritz Barsnick <barsnick@gmx.net> wrote:

> On Wed, Jan 08, 2020 at 15:42:09 +0100, Anton Khirnov wrote:
> > If negative values are allowed, then 'offset' would be more accurate.
>
> That's the word I was looking for, nice. Even valid if only positive
> offsets are allowed. (I understand you implemented wrap-around. Perhaps
> sufficient to write that in the documentation. Or implement both
> directions. ;-))
>
> Moritz
> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavcodec/h265_metadata_bsf.c b/libavcodec/h265_metadata_bsf.c
index b3a1fda144..796046ac7f 100644
--- a/libavcodec/h265_metadata_bsf.c
+++ b/libavcodec/h265_metadata_bsf.c
@@ -68,6 +68,10 @@  typedef struct H265MetadataContext {
     int level;
     int level_guess;
     int level_warned;
+
+    int advance_vps_id;
+    int advance_sps_id;
+    int advance_pps_id;
 } H265MetadataContext;
 
 
@@ -168,6 +172,10 @@  static int h265_metadata_update_vps(AVBSFContext *bsf,
 {
     H265MetadataContext *ctx = bsf->priv_data;
 
+    if(ctx->advance_vps_id > -1) {
+        vps->vps_video_parameter_set_id = (vps->vps_video_parameter_set_id + ctx->advance_vps_id) % HEVC_MAX_VPS_COUNT;
+    }
+
     if (ctx->tick_rate.num && ctx->tick_rate.den) {
         int num, den;
 
@@ -200,6 +208,13 @@  static int h265_metadata_update_sps(AVBSFContext *bsf,
     int need_vui = 0;
     int crop_unit_x, crop_unit_y;
 
+    if(ctx->advance_vps_id > -1) {
+        sps->sps_video_parameter_set_id = (sps->sps_video_parameter_set_id + ctx->advance_vps_id) % HEVC_MAX_VPS_COUNT;
+    }
+    if(ctx->advance_sps_id > -1) {
+        sps->sps_seq_parameter_set_id = (sps->sps_seq_parameter_set_id + ctx->advance_sps_id) % HEVC_MAX_SPS_COUNT;
+    }
+
     if (ctx->sample_aspect_ratio.num && ctx->sample_aspect_ratio.den) {
         // Table E-1.
         static const AVRational sar_idc[] = {
@@ -336,6 +351,32 @@  static int h265_metadata_update_sps(AVBSFContext *bsf,
     return 0;
 }
 
+static int h265_metadata_update_pps(AVBSFContext *bsf,
+                                    H265RawPPS *pps)
+{
+    H265MetadataContext *ctx = bsf->priv_data;
+
+    if(ctx->advance_sps_id > -1) {
+        pps->pps_seq_parameter_set_id = (pps->pps_seq_parameter_set_id + ctx->advance_sps_id) % HEVC_MAX_SPS_COUNT;
+    }
+    if(ctx->advance_pps_id > -1) {
+        pps->pps_pic_parameter_set_id = (pps->pps_pic_parameter_set_id + ctx->advance_pps_id) % HEVC_MAX_PPS_COUNT;
+    }
+
+    return 0;
+}
+
+static int h265_metadata_update_slice_header(AVBSFContext *bsf,
+        H265RawSliceHeader *slice)
+{
+    H265MetadataContext *ctx = bsf->priv_data;
+
+    if(ctx->advance_pps_id > -1) {
+        slice->slice_pic_parameter_set_id = (slice->slice_pic_parameter_set_id + ctx->advance_pps_id) % HEVC_MAX_PPS_COUNT;
+    }
+    return 0;
+}
+
 static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
 {
     H265MetadataContext *ctx = bsf->priv_data;
@@ -406,15 +447,42 @@  static int h265_metadata_filter(AVBSFContext *bsf, AVPacket *pkt)
         h265_metadata_guess_level(bsf, au);
 
     for (i = 0; i < au->nb_units; i++) {
-        if (au->units[i].type == HEVC_NAL_VPS) {
+        switch (au->units[i].type) {
+        case HEVC_NAL_VPS:
             err = h265_metadata_update_vps(bsf, au->units[i].content);
             if (err < 0)
                 goto fail;
-        }
-        if (au->units[i].type == HEVC_NAL_SPS) {
+            break;
+        case HEVC_NAL_SPS:
             err = h265_metadata_update_sps(bsf, au->units[i].content);
             if (err < 0)
                 goto fail;
+            break;
+        case HEVC_NAL_PPS:
+            err = h265_metadata_update_pps(bsf, au->units[i].content);
+            if (err < 0)
+                goto fail;
+            break;
+        case HEVC_NAL_TRAIL_N:
+        case HEVC_NAL_TRAIL_R:
+        case HEVC_NAL_TSA_N:
+        case HEVC_NAL_TSA_R:
+        case HEVC_NAL_STSA_N:
+        case HEVC_NAL_STSA_R:
+        case HEVC_NAL_BLA_W_LP:
+        case HEVC_NAL_BLA_W_RADL:
+        case HEVC_NAL_BLA_N_LP:
+        case HEVC_NAL_IDR_W_RADL:
+        case HEVC_NAL_IDR_N_LP:
+        case HEVC_NAL_CRA_NUT:
+        case HEVC_NAL_RADL_N:
+        case HEVC_NAL_RADL_R:
+        case HEVC_NAL_RASL_N:
+        case HEVC_NAL_RASL_R:
+            err = h265_metadata_update_slice_header(bsf, au->units[i].content);
+            if (err < 0)
+                goto fail;
+            break;
         }
     }
 
@@ -455,15 +523,42 @@  static int h265_metadata_init(AVBSFContext *bsf)
             h265_metadata_guess_level(bsf, au);
 
         for (i = 0; i < au->nb_units; i++) {
-            if (au->units[i].type == HEVC_NAL_VPS) {
+            switch (au->units[i].type) {
+            case HEVC_NAL_VPS:
                 err = h265_metadata_update_vps(bsf, au->units[i].content);
                 if (err < 0)
                     goto fail;
-            }
-            if (au->units[i].type == HEVC_NAL_SPS) {
+                break;
+            case HEVC_NAL_SPS:
                 err = h265_metadata_update_sps(bsf, au->units[i].content);
                 if (err < 0)
                     goto fail;
+                break;
+            case HEVC_NAL_PPS:
+                err = h265_metadata_update_pps(bsf, au->units[i].content);
+                if (err < 0)
+                    goto fail;
+                break;
+            case HEVC_NAL_TRAIL_N:
+            case HEVC_NAL_TRAIL_R:
+            case HEVC_NAL_TSA_N:
+            case HEVC_NAL_TSA_R:
+            case HEVC_NAL_STSA_N:
+            case HEVC_NAL_STSA_R:
+            case HEVC_NAL_BLA_W_LP:
+            case HEVC_NAL_BLA_W_RADL:
+            case HEVC_NAL_BLA_N_LP:
+            case HEVC_NAL_IDR_W_RADL:
+            case HEVC_NAL_IDR_N_LP:
+            case HEVC_NAL_CRA_NUT:
+            case HEVC_NAL_RADL_N:
+            case HEVC_NAL_RADL_R:
+            case HEVC_NAL_RASL_N:
+            case HEVC_NAL_RASL_R:
+                err = h265_metadata_update_slice_header(bsf, au->units[i].content);
+                if (err < 0)
+                    goto fail;
+                break;
             }
         }
 
@@ -547,6 +642,16 @@  static const AVOption h265_metadata_options[] = {
         OFFSET(crop_bottom), AV_OPT_TYPE_INT,
         { .i64 = -1 }, -1, HEVC_MAX_HEIGHT, FLAGS },
 
+    { "advance_vps_id", "Advance the vps id",
+        OFFSET(advance_vps_id), AV_OPT_TYPE_INT,
+        { .i64 = -1 }, -1, HEVC_MAX_VPS_COUNT, FLAGS },
+    {  "advance_sps_id", "Advance the sps id",
+        OFFSET(advance_sps_id), AV_OPT_TYPE_INT,
+        { .i64 = -1 }, -1, HEVC_MAX_SPS_COUNT, FLAGS },
+    {  "advance_pps_id", "Advance the pps id",
+        OFFSET(advance_pps_id), AV_OPT_TYPE_INT,
+        { .i64 = -1 }, -1, HEVC_MAX_PPS_COUNT, FLAGS },
+
     { "level", "Set level (tables A.6 and A.7)",
         OFFSET(level), AV_OPT_TYPE_INT,
         { .i64 = LEVEL_UNSET }, LEVEL_UNSET, 0xff, FLAGS, "level" },