Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-03-03 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Derek Buitenhuis
> Sent: Friday, March 01, 2019 8:43 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support
> for ROI-based encoding
> 
> On 01/03/2019 03:18, Guo, Yejun wrote:
> > yes, that's the reason I pending VP9 work. As for VP8 ROI, another thinking
> > is to first push vp8 roi, since libvpx is an external dependency and we 
> > don't
> > know the time when it is available for vp9 roi. Anyway, I'm open to both.
> 
> Presumably the forthcoming VP9 ROI stuff would have to be under a version
> check anyway, unlike the VP8 path?

yes, I think so. The VP9 ROI path will have a version check to remind people for
the unsupported versions, and the proper time for VP9 roi patch pushing in 
ffmepg
might be when a new libvpx release is available.

> 
> - Derek
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-03-01 Thread James Zern
On Fri, Mar 1, 2019 at 4:51 AM Derek Buitenhuis
 wrote:
>
> On 01/03/2019 03:18, Guo, Yejun wrote:
> > yes, that's the reason I pending VP9 work. As for VP8 ROI, another thinking
> > is to first push vp8 roi, since libvpx is an external dependency and we 
> > don't
> > know the time when it is available for vp9 roi. Anyway, I'm open to both.
>
> Presumably the forthcoming VP9 ROI stuff would have to be under a version
> check anyway, unlike the VP8 path?
>

Yes, that only came about with 1.8.0 (which looks to have at least 1
bug). This can go forward, I didn't expect the discussion to change
anything for vp8, I only thought it might be simpler to serialize
things as the discussion around its use might effect the
implementation here.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-03-01 Thread Derek Buitenhuis
On 01/03/2019 03:18, Guo, Yejun wrote:
> yes, that's the reason I pending VP9 work. As for VP8 ROI, another thinking
> is to first push vp8 roi, since libvpx is an external dependency and we don't
> know the time when it is available for vp9 roi. Anyway, I'm open to both.

Presumably the forthcoming VP9 ROI stuff would have to be under a version
check anyway, unlike the VP8 path?

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-28 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of James Zern
> Sent: Friday, March 01, 2019 9:07 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support
> for ROI-based encoding
> 
> > >
> > > > +if (avctx->codec_id == AV_CODEC_ID_VP8)
> > > > +vp8_encode_set_roi(avctx, frame);
> > >
> > > The API only exists for VP8, or is this due to different quant scales or
> > > something?
> >
> > currently, VP9 ROI does not work, see my discussion at
> https://groups.google.com/a/webmproject.org/forum/#!topic/codec-
> devel/HVBRjoW0fTw,
> > the issue is confirmed by libvpx, and I'm helping to verify their new patch
> for the fix.
> >
> 
> It might be worth letting that conversation finish before pushing
> anything here. The API for VP9 hasn't changed, but there was a bug as
> you pointed out in that thread.
> 

yes, that's the reason I pending VP9 work. As for VP8 ROI, another thinking
is to first push vp8 roi, since libvpx is an external dependency and we don't
know the time when it is available for vp9 roi. Anyway, I'm open to both.

> > It is expected that VP9 ROI support will not be released in a short time, 
> > so I
> just add for vp8 roi.
> > There is something common between VP8/VP9 ROI, and I plan to extract
> an common function when doing the work for VP9 roi.
> >
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-28 Thread James Zern
> >
> > > +if (avctx->codec_id == AV_CODEC_ID_VP8)
> > > +vp8_encode_set_roi(avctx, frame);
> >
> > The API only exists for VP8, or is this due to different quant scales or
> > something?
>
> currently, VP9 ROI does not work, see my discussion at 
> https://groups.google.com/a/webmproject.org/forum/#!topic/codec-devel/HVBRjoW0fTw,
> the issue is confirmed by libvpx, and I'm helping to verify their new patch 
> for the fix.
>

It might be worth letting that conversation finish before pushing
anything here. The API for VP9 hasn't changed, but there was a bug as
you pointed out in that thread.

> It is expected that VP9 ROI support will not be released in a short time, so 
> I just add for vp8 roi.
> There is something common between VP8/VP9 ROI, and I plan to extract an 
> common function when doing the work for VP9 roi.
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-28 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Derek Buitenhuis
> Sent: Thursday, February 28, 2019 1:11 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support
> for ROI-based encoding
> 
> On 27/02/2019 16:12, Guo, Yejun wrote:
> > Signed-off-by: Guo, Yejun 
> > ---
> >  libavcodec/libvpxenc.c | 132
> +
> >  1 file changed, 132 insertions(+)
> 
> Do these APIs exist for all supported libvpx versions?

Yes, I checked release v1.0.0 at https://github.com/webmproject/libvpx/releases,
there is file examples/ vp8_set_maps.txt shows how to set ROI and active maps,
they use the same API as the current API.

> 
> > +if (!sd) {
> > +if (vpx_codec_control(>encoder, VP8E_SET_ACTIVEMAP,
> _map)) {
> > +log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP
> codec control.\n");
> > +return AVERROR_INVALIDDATA;
> > +}
> 
> Why do this stuff at all if no ROIs have been set?

For the case that some middle frames do not have ROI while other frames have 
ROIs.
Due to the fact of libvpx, we have to reset VP8E_SET_ACTIVEMAP, otherwise the 
previous ROIs continue working.
I'll add notes into the code, thanks.

> 
> > +memset(active_map.active_map, 1, active_map.rows *
> active_map.cols);
> 
> Nit: Maybe a comment about what '1' is here.

will add, thanks.

> 
> > +
> > +/* segment id 0 in roi_map is reserved for the areas not covered by
> AVRegionOfInterest.
> > + * segment id 0 in roi_map is also for the areas with
> AVRegionOfInterest.qoffset near 0.
> > + */
> > +segment_id = 0;
> > +segment_mapping[zero_delta_q + MAX_DELTA_Q] = segment_id + 1;
> > +segment_id++;
> 
> This series of ops seems weirdly redundant separately?

yes, I want to show the mapping logic explicitly.  Let me make it simpler.

> 
> > +memset(_map, 0, sizeof(roi_map));
> 
> Can't zero-init?

I guess it not easy to zero-init with " vpx_roi_map_t roi_map = {...};", see 
the struct below.

typedef struct vpx_roi_map {
  /*! If ROI is enabled. */
  uint8_t enabled;
  /*! An id between 0-3 (0-7 for vp9) for each 16x16 (8x8 for VP9)
   * region within a frame. */
  unsigned char *roi_map;
  unsigned int rows; /**< Number of rows. */
  unsigned int cols; /**< Number of columns. */
  /*! VP8 only uses the first 4 segments. VP9 uses 8 segments. */
  int delta_q[8];  /**< Quantizer deltas. */
  int delta_lf[8]; /**< Loop filter deltas. */
  /*! skip and ref frame segment is only used in VP9. */
  int skip[8];  /**< Skip this block. */
  int ref_frame[8]; /**< Reference frame for this block. */
  /*! Static breakout threshold for each segment. Only used in VP8. */
  unsigned int static_threshold[4];
} vpx_roi_map_t;

> 
> > +av_free(active_map.active_map);
> > +av_log(avctx, AV_LOG_ERROR,
> > +   "roi_map alloc (%d bytes) failed.\n",
> > +   roi_map.rows * roi_map.cols);
> 
> Here, and elsewhere: We don't write how many bytes failed, generally.

I use this style because function queue_frames in the same file use it. 
anyway, I'll change it.

> 
> > +if (vpx_codec_control(>encoder, VP8E_SET_ROI_MAP, _map))
> {
> > +av_free(active_map.active_map);
> > +av_free(roi_map.roi_map);
> > +log_encoder_error(avctx, "Failed to set VP8E_SET_ROI_MAP codec
> control.\n");
> > +return AVERROR_INVALIDDATA;
> > +}
> > +
> > +if (vpx_codec_control(>encoder, VP8E_SET_ACTIVEMAP,
> _map)) {
> > +av_free(active_map.active_map);
> > +av_free(roi_map.roi_map);
> > +log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP codec
> control.\n");
> > +return AVERROR_INVALIDDATA;
> > +}
> > +
> > +av_free(active_map.active_map);
> > +av_free(roi_map.roi_map);
> 
> With the amount of failure areas in this function, is it reasonable to roll it
> into one goto fail or something?

good idea, I'll add it.

> 
> > +if (avctx->codec_id == AV_CODEC_ID_VP8)
> > +vp8_encode_set_roi(avctx, frame);
> 
> The API only exists for VP8, or is this due to different quant scales or
> something?

currently, VP9 ROI does not work, see my discussion at 
https://groups.google.com/a/webmproject.org/forum/#!topic/codec-devel/HVBRjoW0fTw,
the issue is confirmed by libvpx, and I'm helping to verify their new patch for 
the fix.

It is ex

Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Mark Thompson
On 27/02/2019 10:22, Gyan wrote:
> On 27-02-2019 01:57 PM, Guo, Yejun wrote:
>>
>>> -Original Message-
>>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
>>> Of Guo, Yejun
>>> Sent: Thursday, February 28, 2019 12:13 AM
>>> To: ffmpeg-devel@ffmpeg.org
>>> Subject: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for
>>> ROI-based encoding
>>>
>>> Signed-off-by: Guo, Yejun 
>>> ---
>>>   libavcodec/libvpxenc.c | 132
>>> +
>>>   1 file changed, 132 insertions(+)
>>>
>>> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
>>> index c823b8a..e3de547 100644
>>> --- a/libavcodec/libvpxenc.c
>>> +++ b/libavcodec/libvpxenc.c
>>> @@ -1057,6 +1057,135 @@ static int queue_frames(AVCodecContext *avctx,
>>> AVPacket *pkt_out)
>>>   return size;
>>>   }
>>>
>>> +static int vp8_encode_set_roi(AVCodecContext *avctx, const AVFrame
>>> *frame)
>>> +{
>>> +    /* range of vpx_roi_map_t.delta_q[i] is [-63, 63] */
>>> +#define MAX_DELTA_Q 63
>>> +
>>> +    AVRegionOfInterest *roi = NULL;
>>> +    vpx_roi_map_t roi_map;
>>> +    AVFrameSideData *sd = av_frame_get_side_data(frame,
>>> AV_FRAME_DATA_REGIONS_OF_INTEREST);
>>> +    VPxContext *ctx = avctx->priv_data;
>>> +    vpx_active_map_t active_map = { 0, 0, 0 };
>>> +    int segment_id;
>>> +    int zero_delta_q = 0;
>>> +
>>> +    /* record the mapping from delta_q to "segment id + 1".
>>> + * delta_q is shift with MAX_DELTA_Q, and so the range is [0,
>>> 2*MAX_DELTA_Q].
>>> + * add 1 to segment id, so no mapping if the value of array element is 
>>> zero.
>>> + */
>>> +    int segment_mapping[2 * MAX_DELTA_Q + 1] = {0};
>>> +
>>> +    active_map.rows = (frame->height + 15) / 16;
>>> +    active_map.cols = (frame->width  + 15) / 16;
>>> +
>>> +    if (!sd) {
>>> +    if (vpx_codec_control(>encoder, VP8E_SET_ACTIVEMAP,
>>> _map)) {
>>> +    log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP 
>>> codec
>>> control.\n");
>>> +    return AVERROR_INVALIDDATA;
>>> +    }
>>> +    return 0;
>>> +    }
>>> +
>>> +    active_map.active_map = av_malloc(active_map.rows * active_map.cols);
>>> +    if (!active_map.active_map) {
>>> +    av_log(avctx, AV_LOG_ERROR,
>>> +   "active_map alloc (%d bytes) failed.\n",
>>> +   active_map.rows * active_map.cols);
>>> +    return AVERROR(ENOMEM);
>>> +    }
>>> +    memset(active_map.active_map, 1, active_map.rows * active_map.cols);
>>> +
>>> +    /* segment id 0 in roi_map is reserved for the areas not covered by
>>> AVRegionOfInterest.
>>> + * segment id 0 in roi_map is also for the areas with
>>> AVRegionOfInterest.qoffset near 0.
>>> + */
>>> +    segment_id = 0;
>>> +    segment_mapping[zero_delta_q + MAX_DELTA_Q] = segment_id + 1;
>>> +    segment_id++;
>>> +    memset(_map, 0, sizeof(roi_map));
>>> +    roi_map.delta_q[segment_id] = zero_delta_q;
>>> +
>>> +    roi_map.rows = active_map.rows;
>>> +    roi_map.cols = active_map.cols;
>>> +    roi_map.roi_map = av_mallocz(roi_map.rows * roi_map.cols);
>>> +    if (!roi_map.roi_map) {
>>> +    av_free(active_map.active_map);
>>> +    av_log(avctx, AV_LOG_ERROR,
>>> +   "roi_map alloc (%d bytes) failed.\n",
>>> +   roi_map.rows * roi_map.cols);
>>> +    return AVERROR(ENOMEM);
>>> +    }
>>> +
>>> +    roi = (AVRegionOfInterest*)sd->data;
>>> +    while ((uint8_t*)roi < sd->data + sd->size) {
>>> +    int qoffset;
>>> +    int mapping_index;
>>> +    int mapping_value;
>>> +    int starty = FFMIN(roi_map.rows, roi->top / 16);
>>> +    int endy   = FFMIN(roi_map.rows, (roi->bottom + 15) / 16);
>>> +    int startx = FFMIN(roi_map.cols, roi->left / 16);
>>> +    int endx   = FFMIN(roi_map.cols, (roi->right + 15) / 16);
>>> +
>>> +    if (roi->self_size == 0) {
>>> +    av_free(active_map.active_map);
>>> +    av_free(roi_map.roi_map);
>>> +    av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size must be
>>> set to sizeof(AVRegionOfInterest).\n");
>>> +    return AVERROR(EINVAL);
>>> +    }
>>> +
>>> +    if (roi->qoffset.den == 0) {
>>> +    av_free(active_map.active_map);
>>> +    av_free(roi_map.roi_map);
>>> +    av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must
>>> not be zero.\n");
>>> +    return AVERROR(EINVAL);
>>> +    }
>>> +
>>> +    qoffset = (int)(roi->qoffset.num * 1.0f / roi->qoffset.den *
>>> MAX_DELTA_Q);
>>> +    qoffset = av_clip(qoffset, -MAX_DELTA_Q, MAX_DELTA_Q);
>>> +
>>> +    mapping_index = qoffset + MAX_DELTA_Q;
>>> +    if (!segment_mapping[mapping_index]) {
>>> +    if (segment_id > 3) {
>>> +    av_log(ctx, AV_LOG_WARNING,
>>> +   "ROI only support 4 segments (and segment 0 is 
>>> reserved for
>>> non-ROIs), skipping this one.\n");
>>> +    roi = 

Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Derek Buitenhuis
On 27/02/2019 17:57, Derek Buitenhuis wrote:
> You're assuming a single ROI, though, which is only one case. Plenty of things
> would need >1.

Also note that sets of ROIs are per-frame.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Derek Buitenhuis
On 27/02/2019 17:31, Gyan wrote:
> Yes, a string. 4 integers and qp offset so either two more, in rational 
> form, or a float.

You're assuming a single ROI, though, which is only one case. Plenty of things
would need >1.

> There's already multiple interfaces accepting long multi-entry 
> multi-valued arguments, whether custom ones in filters like curves or 
> pan, or x264opts, or with the assistance of an existing key-value parser 
> like x264-params. Unless the plan is to allow only machine-generated 
> ROIs, some form of manual user specification will be needed, and that 
> option will have to be prepared to accept and parse such a string.

Feel free to send a patch then I guess... it doesn't sound very appealing
to use them this way, and certaily not to manually add an AVOption for
every codec.

I am not convinced using it manually via CLI options is at all appealing.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Gyan



On 27-02-2019 10:32 PM, Derek Buitenhuis wrote:

On 27/02/2019 13:48, Gyan wrote:

Huh. I haven't suggested removing anything.

I suggested *adding* a way for this feature to be useful for ffmpeg
users in the near-term. Who knows how long will it take for a decent
per-frame ROI filter, like the facedetect example mentioned in the
initial discussion.

Apologies, I misread.

As for an AVOption: That could get ugly, fast. What do you pass it?
A string of ROI and offsets? That would balloon fast.
Yes,  a string. 4 integers and qp offset so either two more, in rational 
form, or a float.


There's already multiple interfaces accepting long multi-entry 
multi-valued arguments, whether custom ones in filters like curves or 
pan, or x264opts, or with the assistance of an existing key-value parser 
like x264-params. Unless the plan is to allow only machine-generated 
ROIs, some form of manual user specification will be needed, and that 
option will have to be prepared to accept and parse such a string.


Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Derek Buitenhuis
On 27/02/2019 16:12, Guo, Yejun wrote:
> Signed-off-by: Guo, Yejun 
> ---
>  libavcodec/libvpxenc.c | 132 
> +
>  1 file changed, 132 insertions(+)

Do these APIs exist for all supported libvpx versions?

> +if (!sd) {
> +if (vpx_codec_control(>encoder, VP8E_SET_ACTIVEMAP, 
> _map)) {
> +log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP codec 
> control.\n");
> +return AVERROR_INVALIDDATA;
> +}

Why do this stuff at all if no ROIs have been set?

> +memset(active_map.active_map, 1, active_map.rows * active_map.cols);

Nit: Maybe a comment about what '1' is here.

> +
> +/* segment id 0 in roi_map is reserved for the areas not covered by 
> AVRegionOfInterest.
> + * segment id 0 in roi_map is also for the areas with 
> AVRegionOfInterest.qoffset near 0.
> + */
> +segment_id = 0;
> +segment_mapping[zero_delta_q + MAX_DELTA_Q] = segment_id + 1;
> +segment_id++;

This series of ops seems weirdly redundant separately?

> +memset(_map, 0, sizeof(roi_map));

Can't zero-init?

> +av_free(active_map.active_map);
> +av_log(avctx, AV_LOG_ERROR,
> +   "roi_map alloc (%d bytes) failed.\n",
> +   roi_map.rows * roi_map.cols);

Here, and elsewhere: We don't write how many bytes failed, generally.

> +if (vpx_codec_control(>encoder, VP8E_SET_ROI_MAP, _map)) {
> +av_free(active_map.active_map);
> +av_free(roi_map.roi_map);
> +log_encoder_error(avctx, "Failed to set VP8E_SET_ROI_MAP codec 
> control.\n");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +if (vpx_codec_control(>encoder, VP8E_SET_ACTIVEMAP, _map)) {
> +av_free(active_map.active_map);
> +av_free(roi_map.roi_map);
> +log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP codec 
> control.\n");
> +return AVERROR_INVALIDDATA;
> +}
> +
> +av_free(active_map.active_map);
> +av_free(roi_map.roi_map);

With the amount of failure areas in this function, is it reasonable to roll it
into one goto fail or something?

> +if (avctx->codec_id == AV_CODEC_ID_VP8)
> +vp8_encode_set_roi(avctx, frame);

The API only exists for VP8, or is this due to different quant scales or 
something?

As an aside, I must say, libvpx's ROI API is real ugly.

Cheers,
- Derek

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Derek Buitenhuis
On 27/02/2019 13:48, Gyan wrote:
> Huh. I haven't suggested removing anything.
> 
> I suggested *adding* a way for this feature to be useful for ffmpeg 
> users in the near-term. Who knows how long will it take for a decent 
> per-frame ROI filter, like the facedetect example mentioned in the 
> initial discussion.

Apologies, I misread.

As for an AVOption: That could get ugly, fast. What do you pass it?
A string of ROI and offsets? That would balloon fast.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Gyan



On 27-02-2019 07:07 PM, Derek Buitenhuis wrote:

On 27/02/2019 10:22, Gyan wrote:

Looks like, at present, the only way to effect ROI is via side data, and
no filter or other in-tree mechanism at present can convey or generate it.

Life exists outside of ffmpeg.c, and it's an extremely useful API to have.


Are there any near-term plans to add some? If not, may I suggest that
you add a private option for supporting encoders, for users to input ROIs?

Please don't suggest removing an API that was *just added*, after many months
of bikeshedding. Please.


Huh. I haven't suggested removing anything.

I suggested *adding* a way for this feature to be useful for ffmpeg 
users in the near-term. Who knows how long will it take for a decent 
per-frame ROI filter, like the facedetect example mentioned in the 
initial discussion.


Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Derek Buitenhuis
On 27/02/2019 10:22, Gyan wrote:
> Looks like, at present, the only way to effect ROI is via side data, and 
> no filter or other in-tree mechanism at present can convey or generate it.

Life exists outside of ffmpeg.c, and it's an extremely useful API to have.

> Are there any near-term plans to add some? If not, may I suggest that 
> you add a private option for supporting encoders, for users to input ROIs?

Please don't suggest removing an API that was *just added*, after many months
of bikeshedding. Please.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Gyan



On 27-02-2019 01:57 PM, Guo, Yejun wrote:



-Original Message-
From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
Of Guo, Yejun
Sent: Thursday, February 28, 2019 12:13 AM
To: ffmpeg-devel@ffmpeg.org
Subject: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for
ROI-based encoding

Signed-off-by: Guo, Yejun 
---
  libavcodec/libvpxenc.c | 132
+
  1 file changed, 132 insertions(+)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index c823b8a..e3de547 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -1057,6 +1057,135 @@ static int queue_frames(AVCodecContext *avctx,
AVPacket *pkt_out)
  return size;
  }

+static int vp8_encode_set_roi(AVCodecContext *avctx, const AVFrame
*frame)
+{
+/* range of vpx_roi_map_t.delta_q[i] is [-63, 63] */
+#define MAX_DELTA_Q 63
+
+AVRegionOfInterest *roi = NULL;
+vpx_roi_map_t roi_map;
+AVFrameSideData *sd = av_frame_get_side_data(frame,
AV_FRAME_DATA_REGIONS_OF_INTEREST);
+VPxContext *ctx = avctx->priv_data;
+vpx_active_map_t active_map = { 0, 0, 0 };
+int segment_id;
+int zero_delta_q = 0;
+
+/* record the mapping from delta_q to "segment id + 1".
+ * delta_q is shift with MAX_DELTA_Q, and so the range is [0,
2*MAX_DELTA_Q].
+ * add 1 to segment id, so no mapping if the value of array element is 
zero.
+ */
+int segment_mapping[2 * MAX_DELTA_Q + 1] = {0};
+
+active_map.rows = (frame->height + 15) / 16;
+active_map.cols = (frame->width  + 15) / 16;
+
+if (!sd) {
+if (vpx_codec_control(>encoder, VP8E_SET_ACTIVEMAP,
_map)) {
+log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP codec
control.\n");
+return AVERROR_INVALIDDATA;
+}
+return 0;
+}
+
+active_map.active_map = av_malloc(active_map.rows * active_map.cols);
+if (!active_map.active_map) {
+av_log(avctx, AV_LOG_ERROR,
+   "active_map alloc (%d bytes) failed.\n",
+   active_map.rows * active_map.cols);
+return AVERROR(ENOMEM);
+}
+memset(active_map.active_map, 1, active_map.rows * active_map.cols);
+
+/* segment id 0 in roi_map is reserved for the areas not covered by
AVRegionOfInterest.
+ * segment id 0 in roi_map is also for the areas with
AVRegionOfInterest.qoffset near 0.
+ */
+segment_id = 0;
+segment_mapping[zero_delta_q + MAX_DELTA_Q] = segment_id + 1;
+segment_id++;
+memset(_map, 0, sizeof(roi_map));
+roi_map.delta_q[segment_id] = zero_delta_q;
+
+roi_map.rows = active_map.rows;
+roi_map.cols = active_map.cols;
+roi_map.roi_map = av_mallocz(roi_map.rows * roi_map.cols);
+if (!roi_map.roi_map) {
+av_free(active_map.active_map);
+av_log(avctx, AV_LOG_ERROR,
+   "roi_map alloc (%d bytes) failed.\n",
+   roi_map.rows * roi_map.cols);
+return AVERROR(ENOMEM);
+}
+
+roi = (AVRegionOfInterest*)sd->data;
+while ((uint8_t*)roi < sd->data + sd->size) {
+int qoffset;
+int mapping_index;
+int mapping_value;
+int starty = FFMIN(roi_map.rows, roi->top / 16);
+int endy   = FFMIN(roi_map.rows, (roi->bottom + 15) / 16);
+int startx = FFMIN(roi_map.cols, roi->left / 16);
+int endx   = FFMIN(roi_map.cols, (roi->right + 15) / 16);
+
+if (roi->self_size == 0) {
+av_free(active_map.active_map);
+av_free(roi_map.roi_map);
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size must be
set to sizeof(AVRegionOfInterest).\n");
+return AVERROR(EINVAL);
+}
+
+if (roi->qoffset.den == 0) {
+av_free(active_map.active_map);
+av_free(roi_map.roi_map);
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must
not be zero.\n");
+return AVERROR(EINVAL);
+}
+
+qoffset = (int)(roi->qoffset.num * 1.0f / roi->qoffset.den *
MAX_DELTA_Q);
+qoffset = av_clip(qoffset, -MAX_DELTA_Q, MAX_DELTA_Q);
+
+mapping_index = qoffset + MAX_DELTA_Q;
+if (!segment_mapping[mapping_index]) {
+if (segment_id > 3) {
+av_log(ctx, AV_LOG_WARNING,
+   "ROI only support 4 segments (and segment 0 is reserved 
for
non-ROIs), skipping this one.\n");
+roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
+continue;
+}
+
+segment_mapping[mapping_index] = segment_id + 1;
+roi_map.delta_q[segment_id] = qoffset;
+segment_id++;
+}
+
+mapping_value = segment_mapping[mapping_index];
+
+for (int y = starty; y < endy; y++)
+for (int x = startx; x < endx; x++)
+roi_map.roi_map[x + y * roi_map.cols] = mapping_value - 1;
+
+roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
+}

Re: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for ROI-based encoding

2019-02-27 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Guo, Yejun
> Sent: Thursday, February 28, 2019 12:13 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH] avcodec/libvpxenc: add VP8 support for
> ROI-based encoding
> 
> Signed-off-by: Guo, Yejun 
> ---
>  libavcodec/libvpxenc.c | 132
> +
>  1 file changed, 132 insertions(+)
> 
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index c823b8a..e3de547 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -1057,6 +1057,135 @@ static int queue_frames(AVCodecContext *avctx,
> AVPacket *pkt_out)
>  return size;
>  }
> 
> +static int vp8_encode_set_roi(AVCodecContext *avctx, const AVFrame
> *frame)
> +{
> +/* range of vpx_roi_map_t.delta_q[i] is [-63, 63] */
> +#define MAX_DELTA_Q 63
> +
> +AVRegionOfInterest *roi = NULL;
> +vpx_roi_map_t roi_map;
> +AVFrameSideData *sd = av_frame_get_side_data(frame,
> AV_FRAME_DATA_REGIONS_OF_INTEREST);
> +VPxContext *ctx = avctx->priv_data;
> +vpx_active_map_t active_map = { 0, 0, 0 };
> +int segment_id;
> +int zero_delta_q = 0;
> +
> +/* record the mapping from delta_q to "segment id + 1".
> + * delta_q is shift with MAX_DELTA_Q, and so the range is [0,
> 2*MAX_DELTA_Q].
> + * add 1 to segment id, so no mapping if the value of array element is 
> zero.
> + */
> +int segment_mapping[2 * MAX_DELTA_Q + 1] = {0};
> +
> +active_map.rows = (frame->height + 15) / 16;
> +active_map.cols = (frame->width  + 15) / 16;
> +
> +if (!sd) {
> +if (vpx_codec_control(>encoder, VP8E_SET_ACTIVEMAP,
> _map)) {
> +log_encoder_error(avctx, "Failed to set VP8E_SET_ACTIVEMAP codec
> control.\n");
> +return AVERROR_INVALIDDATA;
> +}
> +return 0;
> +}
> +
> +active_map.active_map = av_malloc(active_map.rows * active_map.cols);
> +if (!active_map.active_map) {
> +av_log(avctx, AV_LOG_ERROR,
> +   "active_map alloc (%d bytes) failed.\n",
> +   active_map.rows * active_map.cols);
> +return AVERROR(ENOMEM);
> +}
> +memset(active_map.active_map, 1, active_map.rows * active_map.cols);
> +
> +/* segment id 0 in roi_map is reserved for the areas not covered by
> AVRegionOfInterest.
> + * segment id 0 in roi_map is also for the areas with
> AVRegionOfInterest.qoffset near 0.
> + */
> +segment_id = 0;
> +segment_mapping[zero_delta_q + MAX_DELTA_Q] = segment_id + 1;
> +segment_id++;
> +memset(_map, 0, sizeof(roi_map));
> +roi_map.delta_q[segment_id] = zero_delta_q;
> +
> +roi_map.rows = active_map.rows;
> +roi_map.cols = active_map.cols;
> +roi_map.roi_map = av_mallocz(roi_map.rows * roi_map.cols);
> +if (!roi_map.roi_map) {
> +av_free(active_map.active_map);
> +av_log(avctx, AV_LOG_ERROR,
> +   "roi_map alloc (%d bytes) failed.\n",
> +   roi_map.rows * roi_map.cols);
> +return AVERROR(ENOMEM);
> +}
> +
> +roi = (AVRegionOfInterest*)sd->data;
> +while ((uint8_t*)roi < sd->data + sd->size) {
> +int qoffset;
> +int mapping_index;
> +int mapping_value;
> +int starty = FFMIN(roi_map.rows, roi->top / 16);
> +int endy   = FFMIN(roi_map.rows, (roi->bottom + 15) / 16);
> +int startx = FFMIN(roi_map.cols, roi->left / 16);
> +int endx   = FFMIN(roi_map.cols, (roi->right + 15) / 16);
> +
> +if (roi->self_size == 0) {
> +av_free(active_map.active_map);
> +av_free(roi_map.roi_map);
> +av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.self_size must be
> set to sizeof(AVRegionOfInterest).\n");
> +return AVERROR(EINVAL);
> +}
> +
> +if (roi->qoffset.den == 0) {
> +av_free(active_map.active_map);
> +av_free(roi_map.roi_map);
> +av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must
> not be zero.\n");
> +return AVERROR(EINVAL);
> +}
> +
> +qoffset = (int)(roi->qoffset.num * 1.0f / roi->qoffset.den *
> MAX_DELTA_Q);
> +qoffset = av_clip(qoffset, -MAX_DELTA_Q, MAX_DELTA_Q);
> +
> +mapping_index = qoffset + MAX_DELTA_Q;
> +if (!segment_mapping[mapping_index]) {
> +if (segment_id > 3) {
> +av_log(ctx, AV_LOG_WARNING,
> +   "ROI only support 4 segments (and segment 0 is 
> reserved for
> non-ROIs), skipping this one.\n");
> +roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
> +continue;
> +}
> +
> +segment_mapping[mapping_index] = segment_id + 1;
> +roi_map.delta_q[segment_id] = qoffset;
> +segment_id++;
> +}
> +
> +mapping_value = segment_mapping[mapping_index];
> +
> +for (int