| Message ID | 1551283950-931-1-git-send-email-yejun.guo@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@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 <yejun.guo@intel.com> > --- > 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(&ctx->encoder, VP8E_SET_ACTIVEMAP, > &active_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(&roi_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); > + } > + > + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ROI_MAP, &roi_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(&ctx->encoder, VP8E_SET_ACTIVEMAP, > &active_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); > + > + return 0; > +} > + > static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, > const AVFrame *frame, int *got_packet) > { > @@ -1113,6 +1242,9 @@ static int vpx_encode(AVCodecContext *avctx, > AVPacket *pkt, > flags |= strtoul(en->value, NULL, 10); > } > } > + > + if (avctx->codec_id == AV_CODEC_ID_VP8) > + vp8_encode_set_roi(avctx, frame); > } > > res = vpx_codec_encode(&ctx->encoder, rawimg, timestamp, to verify this patch, please cherry-pick https://github.com/guoyejun/ffmpeg/commit/7ba191ad7da5cfd1e6241d0a549c3c6b88c6b2f8, and run command like: ./ffmpeg -i .../path_to_1920x1080_video_file -vf scale=1920:1080 -c:v libvpx -b:v 5M -y tmp.webm btw, I plan to add an example under doc/examples/ for the ROI encoding together with an object detection filter for ffmpeg. It's my next task to do. > -- > 2.7.4 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 27-02-2019 01:57 PM, Guo, Yejun wrote: > >> -----Original Message----- >> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@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 <yejun.guo@intel.com> >> --- >> 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(&ctx->encoder, VP8E_SET_ACTIVEMAP, >> &active_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(&roi_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); >> + } >> + >> + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ROI_MAP, &roi_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(&ctx->encoder, VP8E_SET_ACTIVEMAP, >> &active_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); >> + >> + return 0; >> +} >> + >> static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, >> const AVFrame *frame, int *got_packet) >> { >> @@ -1113,6 +1242,9 @@ static int vpx_encode(AVCodecContext *avctx, >> AVPacket *pkt, >> flags |= strtoul(en->value, NULL, 10); >> } >> } >> + >> + if (avctx->codec_id == AV_CODEC_ID_VP8) >> + vp8_encode_set_roi(avctx, frame); >> } >> >> res = vpx_codec_encode(&ctx->encoder, rawimg, timestamp, > to verify this patch, please cherry-pick https://github.com/guoyejun/ffmpeg/commit/7ba191ad7da5cfd1e6241d0a549c3c6b88c6b2f8, > and run command like: > ./ffmpeg -i .../path_to_1920x1080_video_file -vf scale=1920:1080 -c:v libvpx -b:v 5M -y tmp.webm > 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. 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? Thanks, Gyan
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
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
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
On 27/02/2019 16:12, Guo, Yejun wrote: > Signed-off-by: Guo, Yejun <yejun.guo@intel.com> > --- > libavcodec/libvpxenc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 132 insertions(+) Do these APIs exist for all supported libvpx versions? > + if (!sd) { > + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ACTIVEMAP, &active_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(&roi_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(&ctx->encoder, VP8E_SET_ROI_MAP, &roi_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(&ctx->encoder, VP8E_SET_ACTIVEMAP, &active_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
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
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
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
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-bounces@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 <yejun.guo@intel.com> >>> --- >>> 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(&ctx->encoder, VP8E_SET_ACTIVEMAP, >>> &active_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(&roi_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); >>> + } >>> + >>> + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ROI_MAP, &roi_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(&ctx->encoder, VP8E_SET_ACTIVEMAP, >>> &active_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); >>> + >>> + return 0; >>> +} >>> + >>> static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, >>> const AVFrame *frame, int *got_packet) >>> { >>> @@ -1113,6 +1242,9 @@ static int vpx_encode(AVCodecContext *avctx, >>> AVPacket *pkt, >>> flags |= strtoul(en->value, NULL, 10); >>> } >>> } >>> + >>> + if (avctx->codec_id == AV_CODEC_ID_VP8) >>> + vp8_encode_set_roi(avctx, frame); >>> } >>> >>> res = vpx_codec_encode(&ctx->encoder, rawimg, timestamp, >> to verify this patch, please cherry-pick https://github.com/guoyejun/ffmpeg/commit/7ba191ad7da5cfd1e6241d0a549c3c6b88c6b2f8, >> and run command like: >> ./ffmpeg -i .../path_to_1920x1080_video_file -vf scale=1920:1080 -c:v libvpx -b:v 5M -y tmp.webm >> > > 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. > > 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? I had a filter doing exactly this for testing - it adds the ROI side-data to frames, and can be applied multiple times to add multiple regions. I've included it the set just sent, though I'm not sure how useful it actually is for non-test cases. See <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2019-February/240533.html>. Thanks, - Mark
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@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 <yejun.guo@intel.com> > > --- > > 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(&ctx->encoder, VP8E_SET_ACTIVEMAP, > &active_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(&roi_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(&ctx->encoder, VP8E_SET_ROI_MAP, &roi_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(&ctx->encoder, VP8E_SET_ACTIVEMAP, > &active_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 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. btw, I'll also change the patch a little on how to iterate the AVRegionOfInterest array, because I think Mark Thompson's code is better than mine. > > 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
> > > > > + 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. >
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@ffmpeg.org] On Behalf > Of James Zern > Sent: Friday, March 01, 2019 9:07 AM > To: FFmpeg development discussions and patches <ffmpeg- > devel@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
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
On Fri, Mar 1, 2019 at 4:51 AM Derek Buitenhuis <derek.buitenhuis@gmail.com> 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.
> -----Original Message----- > From: ffmpeg-devel [mailto:ffmpeg-devel-bounces@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
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(&ctx->encoder, VP8E_SET_ACTIVEMAP, &active_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(&roi_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); + } + + if (vpx_codec_control(&ctx->encoder, VP8E_SET_ROI_MAP, &roi_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(&ctx->encoder, VP8E_SET_ACTIVEMAP, &active_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); + + return 0; +} + static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, const AVFrame *frame, int *got_packet) { @@ -1113,6 +1242,9 @@ static int vpx_encode(AVCodecContext *avctx, AVPacket *pkt, flags |= strtoul(en->value, NULL, 10); } } + + if (avctx->codec_id == AV_CODEC_ID_VP8) + vp8_encode_set_roi(avctx, frame); } res = vpx_codec_encode(&ctx->encoder, rawimg, timestamp,
Signed-off-by: Guo, Yejun <yejun.guo@intel.com> --- libavcodec/libvpxenc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+)