[FFmpeg-devel] [PATCH 4/6] avcodec/hevcdec: make set_side_data() accessible

[Soft Works]

> -----Original Message-----
> From: Xiang, Haihao <haihao.xiang at intel.com>
> Sent: Tuesday, May 31, 2022 11:38 AM
> To: ffmpeg-devel at ffmpeg.org
> Cc: softworkz at hotmail.com
> Subject: Re: [FFmpeg-devel] [PATCH 4/6] avcodec/hevcdec: make
> set_side_data() accessible
> 
> On Thu, 2022-05-26 at 08:08 +0000, softworkz wrote:
> > From: softworkz <softworkz at hotmail.com>
> >
> > Signed-off-by: softworkz <softworkz at hotmail.com>
> > ---
> >  libavcodec/hevcdec.c | 117 +++++++++++++++++++++------------------
> ----
> >  libavcodec/hevcdec.h |   2 +
> >  2 files changed, 60 insertions(+), 59 deletions(-)
> >
> > diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> > index f782ea6394..ff22081b46 100644
> > --- a/libavcodec/hevcdec.c
> > +++ b/libavcodec/hevcdec.c
> > @@ -2726,23 +2726,22 @@ error:
> >      return res;
> >  }
> >
> > -static int set_side_data(HEVCContext *s)
> > +int ff_set_side_data(AVCodecContext *logctx, HEVCSEI *sei,
> HEVCContext *s,
> > AVFrame *out)
> >  {
> > -    AVFrame *out = s->ref->frame;
> > -    int ret;
> > +    int ret = 0;
> >
> > -    if (s->sei.frame_packing.present &&
> > -        s->sei.frame_packing.arrangement_type >= 3 &&
> > -        s->sei.frame_packing.arrangement_type <= 5 &&
> > -        s->sei.frame_packing.content_interpretation_type > 0 &&
> > -        s->sei.frame_packing.content_interpretation_type < 3) {
> > +    if (sei->frame_packing.present &&
> > +        sei->frame_packing.arrangement_type >= 3 &&
> > +        sei->frame_packing.arrangement_type <= 5 &&
> > +        sei->frame_packing.content_interpretation_type > 0 &&
> > +        sei->frame_packing.content_interpretation_type < 3) {
> >          AVStereo3D *stereo = av_stereo3d_create_side_data(out);
> >          if (!stereo)
> >              return AVERROR(ENOMEM);
> >
> > -        switch (s->sei.frame_packing.arrangement_type) {
> > +        switch (sei->frame_packing.arrangement_type) {
> >          case 3:
> > -            if (s->sei.frame_packing.quincunx_subsampling)
> > +            if (sei->frame_packing.quincunx_subsampling)
> >                  stereo->type = AV_STEREO3D_SIDEBYSIDE_QUINCUNX;
> >              else
> >                  stereo->type = AV_STEREO3D_SIDEBYSIDE;
> > @@ -2755,21 +2754,21 @@ static int set_side_data(HEVCContext *s)
> >              break;
> >          }
> >
> > -        if (s->sei.frame_packing.content_interpretation_type == 2)
> > +        if (sei->frame_packing.content_interpretation_type == 2)
> >              stereo->flags = AV_STEREO3D_FLAG_INVERT;
> >
> > -        if (s->sei.frame_packing.arrangement_type == 5) {
> > -            if (s->sei.frame_packing.current_frame_is_frame0_flag)
> > +        if (sei->frame_packing.arrangement_type == 5) {
> > +            if (sei->frame_packing.current_frame_is_frame0_flag)
> >                  stereo->view = AV_STEREO3D_VIEW_LEFT;
> >              else
> >                  stereo->view = AV_STEREO3D_VIEW_RIGHT;
> >          }
> >      }
> >
> > -    if (s->sei.display_orientation.present &&
> > -        (s->sei.display_orientation.anticlockwise_rotation ||
> > -         s->sei.display_orientation.hflip || s-
> > >sei.display_orientation.vflip)) {
> > -        double angle = s-
> >sei.display_orientation.anticlockwise_rotation *
> > 360 / (double) (1 << 16);
> > +    if (sei->display_orientation.present &&
> > +        (sei->display_orientation.anticlockwise_rotation ||
> > +         sei->display_orientation.hflip || sei-
> >display_orientation.vflip)) {
> > +        double angle = sei-
> >display_orientation.anticlockwise_rotation * 360
> > / (double) (1 << 16);
> >          AVFrameSideData *rotation = av_frame_new_side_data(out,
> >
> AV_FRAME_DATA_DISP
> > LAYMATRIX,
> >
> sizeof(int32_t) *
> > 9);
> > @@ -2788,17 +2787,17 @@ static int set_side_data(HEVCContext *s)
> >                         * (1 - 2 * !!s-
> >sei.display_orientation.vflip);
> >          av_display_rotation_set((int32_t *)rotation->data, angle);
> >          av_display_matrix_flip((int32_t *)rotation->data,
> > -                               s->sei.display_orientation.hflip,
> > -                               s->sei.display_orientation.vflip);
> > +                               sei->display_orientation.hflip,
> > +                               sei->display_orientation.vflip);
> >      }
> >
> >      // Decrement the mastering display flag when IRAP frame has
> > no_rasl_output_flag=1
> >      // so the side data persists for the entire coded video
> sequence.
> > -    if (s->sei.mastering_display.present > 0 &&
> > +    if (s && sei->mastering_display.present > 0 &&
> 
> So sei must be non-NULL but s may be NULL in the new function, right
> ? 

Right.

> The
> caller should ensure s is non-NULL for the original function. It
> would be better
> to have some comment if s may be NULL now.

Ok, I will add that comment.

In a future update, I would like to further extend this to allow
parsing of other SEI which currently still requires a non-null
HEVCContext (e.g. DOVI parsing), but I figured that this would 
go too far for doing this as part of this patchset and I'm not 
sure about the best way to do that, means either:

- Mocking an HEVCContext
  or
- Changing the individual functions to not require an HEVCContext

So, for now, I'll add the comment.


> > -static int set_side_data(HEVCContext *s)
> > +int ff_set_side_data(AVCodecContext *logctx, HEVCSEI *sei,
> HEVCContext *s,
> 
> How about to use ff_hevc as prefix ? This function is used for hevc
> only

Yes makes sense, will change.


Thanks for reviewing,
softworkz