[FFmpeg-devel] [PATCH V4 1/2] avutil: add ROI data struct and bump version

Guo, Yejun yejun.guo at intel.com
Thu Jan 3 07:42:25 EET 2019



> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf
> Of Vittorio Giovara
> Sent: Thursday, January 03, 2019 4:55 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V4 1/2] avutil: add ROI data struct and
> bump version
> 
> On Wed, Jan 2, 2019 at 6:48 PM James Almer <jamrial at gmail.com> wrote:
> 
> > On 12/28/2018 7:09 AM, Guo, Yejun wrote:
> > > The encoders such as libx264 support different QPs offset for
> > > different
> > MBs,
> > > it makes possible for ROI-based encoding. It makes sense to add
> > > support within ffmpeg to generate/accept ROI infos and pass into
> encoders.
> > >
> > > Typical usage: After AVFrame is decoded, a ffmpeg filter or user's
> > > code generates ROI info for that frame, and the encoder finally does
> > > the ROI-based encoding.
> > >
> > > The ROI info is maintained as side data of AVFrame.
> > >
> > > Signed-off-by: Guo, Yejun <yejun.guo at intel.com>
> > > ---
> > >  libavutil/frame.c   |  1 +
> > >  libavutil/frame.h   | 23 +++++++++++++++++++++++
> > >  libavutil/version.h |  2 +-
> > >  3 files changed, 25 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavutil/frame.c b/libavutil/frame.c index
> > > 34a6210..bebc50e 100644
> > > --- a/libavutil/frame.c
> > > +++ b/libavutil/frame.c
> > > @@ -841,6 +841,7 @@ const char *av_frame_side_data_name(enum
> > AVFrameSideDataType type)
> > >      case AV_FRAME_DATA_QP_TABLE_DATA:               return "QP table
> > data";
> > >  #endif
> > >      case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic
> > > Metadata
> > SMPTE2094-40 (HDR10+)";
> > > +    case AV_FRAME_DATA_ROIS: return "Regions Of Interest";
> > >      }
> > >      return NULL;
> > >  }
> > > diff --git a/libavutil/frame.h b/libavutil/frame.h index
> > > 582ac47..3460b01 100644
> > > --- a/libavutil/frame.h
> > > +++ b/libavutil/frame.h
> > > @@ -173,6 +173,12 @@ enum AVFrameSideDataType {
> > >       * volume transform - application 4 of SMPTE 2094-40:2016 standard.
> > >       */
> > >      AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> > > +
> > > +    /**
> > > +     * Regions Of Interest, the data is an array of AVROI type, the
> > array size
> > > +     * is implied by AVFrameSideData::size / sizeof(AVROI).
> > > +     */
> > > +    AV_FRAME_DATA_ROIS,
> > >  };
> > >
> > >  enum AVActiveFormatDescription {
> > > @@ -201,6 +207,23 @@ typedef struct AVFrameSideData {  }
> > > AVFrameSideData;
> > >
> > >  /**
> > > + * Structure to hold Region Of Interest.
> > > + *
> > > + * top/bottom/left/right are coordinates at frame pixel level.
> > > + * They will be extended internally if the codec requires an alignment.
> > > + * If the regions overlap, the last value in the list will be used.
> > > + *
> > > + * qoffset is quant offset, it is encoder dependent.
> > > + */
> > > +typedef struct AVROI {
> > > +    size_t top;
> > > +    size_t bottom;
> > > +    size_t left;
> > > +    size_t right;
> >
> > uint32_t, please. Make the struct have a fixed size so we don't repeat
> > the same issues we had with fate tests and AVSphericalMapping.
> >
> 
> I thought we dropped the side data size from fate tests in
> 21a8e751ad6abb2d423afa3041da92f8f7741997.
> If so, size_t seems the better choice here.

I usually choose 'size_t' for the meanings with length/size. Will keep this since 21a8e751ad6abb2d423afa3041da92f8f7741997 was there.

> --
> Vittorio
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list