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

2019-02-28 Thread Guo, Yejun
Signed-off-by: Guo, Yejun 
---
 libavcodec/libvpxenc.c | 150 +
 1 file changed, 150 insertions(+)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index c823b8a..fc9b1fd 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -1057,6 +1057,153 @@ 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
+
+const AVRegionOfInterest *roi = NULL;
+vpx_roi_map_t roi_map;
+int ret = 0;
+int nb_rois;
+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 max_segment_cnt = 4;/* VP8 ROI only support 4 segments. */
+int segment_id = 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};
+
+/* 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_mapping[MAX_DELTA_Q] = segment_id + 1;
+segment_id++;
+
+active_map.rows = (frame->height + 15) / 16;
+active_map.cols = (frame->width  + 15) / 16;
+
+if (!sd) {
+/* For the case that some middle frames do not have ROI while other 
frames have ROIs.
+ * Due to libvpx behaivor, we have to reset VP8E_SET_ACTIVEMAP, 
otherwise the previous
+ * ROIs continue working for frames without ROIs.
+ */
+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;
+}
+
+memset(_map, 0, sizeof(roi_map));
+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_log(avctx, AV_LOG_ERROR, "roi_map alloc failed.\n");
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+
+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 failed.\n");
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+/* set 1 to enable the corresponding element of vpx_roi_map_t.roi_map. */
+memset(active_map.active_map, 1, active_map.rows * active_map.cols);
+
+roi = (const AVRegionOfInterest*)sd->data;
+if (!roi->self_size || sd->size % roi->self_size != 0) {
+av_log(ctx, AV_LOG_ERROR, "Invalid AVRegionOfInterest.self_size.\n");
+ret = AVERROR(EINVAL);
+goto fail;
+}
+nb_rois = sd->size / roi->self_size;
+
+/* This list must be iterated from zero because regions are
+ * defined in order of decreasing importance. So discard less
+ * important areas if exceeding the supported segment count.
+ */
+for (int i = 0; i < nb_rois; i++) {
+int qoffset;
+int mapping_index;
+
+roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i);
+if (roi->qoffset.den == 0) {
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must not 
be zero.\n");
+ret = AVERROR(EINVAL);
+goto fail;
+}
+
+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 > max_segment_cnt - 1) {
+av_log(ctx, AV_LOG_WARNING,
+   "ROI only support %d segments (and segment 0 is 
reserved for non-ROIs), skipping this one.\n",
+   max_segment_cnt);
+roi = (AVRegionOfInterest*)((char*)roi + roi->self_size);
+continue;
+}
+
+segment_mapping[mapping_index] = segment_id + 1;
+roi_map.delta_q[segment_id] = qoffset;
+segment_id++;
+}
+}
+
+/* This list must be iterated in reverse, so for the case that
+ * two regions overlapping, the more important area takes effect.
+ */
+for (int i = nb_rois - 1; i >= 0; i--) {
+int qoffset;
+int mapping_value;
+int starty, endy, startx, endx;
+
+roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i);
+
+starty = FFMIN(roi_map.rows, roi->top / 16);
+endy   = FFMIN(roi_map.rows, 

[FFmpeg-devel] [PATCH] lavf/hlsdec: fix support for large initialization segments

2019-02-28 Thread Rodger Combs
---
 libavformat/hls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index fc9110356d..c56ead507b 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1553,7 +1553,7 @@ reload:
 if (v->init_sec_buf_read_offset < v->init_sec_data_len) {
 /* Push init section out first before first actual segment */
 int copy_size = FFMIN(v->init_sec_data_len - 
v->init_sec_buf_read_offset, buf_size);
-memcpy(buf, v->init_sec_buf, copy_size);
+memcpy(buf, v->init_sec_buf + v->init_sec_buf_read_offset, copy_size);
 v->init_sec_buf_read_offset += copy_size;
 return copy_size;
 }
-- 
2.20.1

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


Re: [FFmpeg-devel] [PATCH V2] 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 Guo, Yejun
> Sent: Friday, March 01, 2019 3:10 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH V2] avcodec/libvpxenc: add VP8 support for
> ROI-based encoding
> 
> Signed-off-by: Guo, Yejun 
> ---
>  libavcodec/libvpxenc.c | 131
> +
>  1 file changed, 131 insertions(+)
> 
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index c823b8a..0f8efcc 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -1057,6 +1057,134 @@ 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
> +
> +const AVRegionOfInterest *roi = NULL;
> +vpx_roi_map_t roi_map;
> +int ret = 0;
> +int nb_rois;
> +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 = 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};
> +
> +/* 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_mapping[MAX_DELTA_Q] = segment_id + 1;
> +segment_id++;
> +
> +active_map.rows = (frame->height + 15) / 16;
> +active_map.cols = (frame->width  + 15) / 16;
> +
> +if (!sd) {
> +/* For the case that some middle frames do not have ROI while other
> frames have ROIs.
> + * Due to libvpx behaivor, we have to reset VP8E_SET_ACTIVEMAP,
> otherwise the previous
> + * ROIs continue working for frames without ROIs.
> + */
> +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;
> +}
> +
> +memset(_map, 0, sizeof(roi_map));
> +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_log(avctx, AV_LOG_ERROR, "roi_map alloc failed.\n");
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
> +
> +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 failed.\n");
> +ret = AVERROR(ENOMEM);
> +goto fail;
> +}
> +/* set 1 to enable the corresponding element of vpx_roi_map_t.roi_map.
> */
> +memset(active_map.active_map, 1, active_map.rows * active_map.cols);
> +
> +roi = (const AVRegionOfInterest*)sd->data;
> +if (!roi->self_size || sd->size % roi->self_size != 0) {
> +av_log(ctx, AV_LOG_ERROR, "Invalid AVRegionOfInterest.self_size.\n");
> +ret = AVERROR(EINVAL);
> +goto fail;
> +}
> +nb_rois = sd->size / roi->self_size;
> +
> +// This list must be iterated in reverse because regions are
> +// defined in order of decreasing importance.
> +for (int i = nb_rois - 1; i >= 0; i--) {
> +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);
> +
> +roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i);
> +if (roi->qoffset.den == 0) {
> +av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must
> not be zero.\n");
> +ret = AVERROR(EINVAL);
> +goto fail;
> +}
> +
> +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;

please ignore this patch v2, because the most 

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 4/5] vaapi_encode: Add ROI support

2019-02-28 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of Michael Niedermayer
> Sent: Friday, March 01, 2019 6:13 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support
> 
> On Wed, Feb 27, 2019 at 10:00:22PM +, Mark Thompson wrote:
> > ---
> >  libavcodec/vaapi_encode.c   | 129
> 
> >  libavcodec/vaapi_encode.h   |   9 +++
> >  libavcodec/vaapi_encode_h264.c  |   2 +
> >  libavcodec/vaapi_encode_h265.c  |   2 +
> >  libavcodec/vaapi_encode_mpeg2.c |   2 +
> >  libavcodec/vaapi_encode_vp8.c   |   2 +
> >  libavcodec/vaapi_encode_vp9.c   |   2 +
> >  7 files changed, 148 insertions(+)
> 
> In file included from libavcodec/vaapi_encode.c:27:0:
> libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’
>  VAEncROI   *roi;
>  ^
> make: *** [libavcodec/vaapi_encode.o] Error 1

looks that we'd add a libva version check, this struct is defined at libva 
repo. 
./va/va.h:2259:} VAEncROI;

I yesterday just did 'git pull' for the following repos:
https://github.com/intel/libva
https://github.com/intel/gmmlib.git
https://github.com/intel/media-driver.git 


> make: *** Waiting for unfinished jobs
> In file included from libavcodec/vaapi_encode_mpeg2.c:28:0:
> libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’
>  VAEncROI   *roi;
>  ^
> make: *** [libavcodec/vaapi_encode_mpeg2.o] Error 1
> POD   doc/ffprobe.pod
> In file included from libavcodec/vaapi_encode_h264.c:36:0:
> libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’
>  VAEncROI   *roi;
>  ^
> make: *** [libavcodec/vaapi_encode_h264.o] Error 1
> 
> 
> 
> [...]
> --
> Michael GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> Dictatorship: All citizens are under surveillance, all their steps and
> actions recorded, for the politicians to enforce control.
> Democracy: All politicians are under surveillance, all their steps and
> actions recorded, for the citizens to enforce control.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] avformat/westwood_aud: Adds PCM format demux.

2019-02-28 Thread Michael Niedermayer
On Wed, Feb 27, 2019 at 11:12:08PM +, Aidan R wrote:
> PCM format AUD files are found in Westwood's Blade Runner game.
> ---
>  libavformat/westwood_aud.c | 80 
> --
>  1 file changed, 63 insertions(+), 17 deletions(-)
> 
> diff --git a/libavformat/westwood_aud.c b/libavformat/westwood_aud.c
> index 9c2d35cb8a..5d7e827bc1 100644
> --- a/libavformat/westwood_aud.c
> +++ b/libavformat/westwood_aud.c
> @@ -41,6 +41,12 @@
>  #define AUD_HEADER_SIZE 12
>  #define AUD_CHUNK_PREAMBLE_SIZE 8
>  #define AUD_CHUNK_SIGNATURE 0xDEAF
> +#define AUD_PCM_CHUNK_SIZE 2048 /* arbitrary size to pull out of PCM data*/
> +
> +typedef struct AUDDemuxContext {
> +int size;
> +int pos;
> +} AUDDemuxContext;
>  
>  static int wsaud_probe(AVProbeData *p)
>  {
> @@ -50,10 +56,10 @@ static int wsaud_probe(AVProbeData *p)
>   * so perform sanity checks on various header parameters:
>   *   8000 <= sample rate (16 bits) <= 48000  ==> 40001 acceptable numbers
>   *   flags <= 0x03 (2 LSBs are used) ==> 4 acceptable numbers
> - *   compression type (8 bits) = 1 or 99 ==> 2 acceptable numbers
> + *   compression type (8 bits) = 0, 1 or 99  ==> 3 acceptable numbers
>   *   first audio chunk signature (32 bits)   ==> 1 acceptable number
> - * The number space contains 2^64 numbers. There are 40001 * 4 * 2 * 1 =
> - * 320008 acceptable number combinations.
> + * The number space contains 2^64 numbers. There are 40001 * 4 * 3 * 1 =
> + * 480012 acceptable number combinations.
>   */
>  
>  if (p->buf_size < AUD_HEADER_SIZE + AUD_CHUNK_PREAMBLE_SIZE)
> @@ -69,13 +75,22 @@ static int wsaud_probe(AVProbeData *p)
>  if (p->buf[10] & 0xFC)
>  return 0;
>  
> -if (p->buf[11] != 99 && p->buf[11] != 1)
> +/* valid format values are 99 == adpcm, 1 == snd1 and 0 == pcm */
> +if (p->buf[11] != 99 && p->buf[11] != 1 && p->buf[11] != 0)
>  return 0;
>  
> -/* read ahead to the first audio chunk and validate the first header 
> signature */
> -if (AV_RL32(>buf[16]) != AUD_CHUNK_SIGNATURE)
> +/* read ahead to the first audio chunk and validate the first header
> + * signature pcm format does not use a chunk format, so don't check
> + * for that codec */
> +if (p->buf[11] != 0 && AV_RL32(>buf[16]) != AUD_CHUNK_SIGNATURE)
>  return 0;
>  
> +/* if we have pcm format then compressed size should equal
> + * uncompressed size */
> +if (p->buf[11] == 0 && AV_RL32(>buf[2]) != AV_RL32(>buf[6]))  {
> +return 0;
> +}
> +
>  /* return 1/2 certainty since this file check is a little sketchy */
>  return AVPROBE_SCORE_EXTENSION;
>  }

this seems to break probetest
tools/probetest 256 1024
testing size=1
testing size=2
testing size=4
testing size=8
testing size=16
testing size=32
Failure of wsaud probing code with score=50 type=0 p=EBC size=32
testing size=64
testing size=128
testing size=256
testing size=512

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 4/5] vaapi_encode: Add ROI support

2019-02-28 Thread Michael Niedermayer
On Wed, Feb 27, 2019 at 10:00:22PM +, Mark Thompson wrote:
> ---
>  libavcodec/vaapi_encode.c   | 129 
>  libavcodec/vaapi_encode.h   |   9 +++
>  libavcodec/vaapi_encode_h264.c  |   2 +
>  libavcodec/vaapi_encode_h265.c  |   2 +
>  libavcodec/vaapi_encode_mpeg2.c |   2 +
>  libavcodec/vaapi_encode_vp8.c   |   2 +
>  libavcodec/vaapi_encode_vp9.c   |   2 +
>  7 files changed, 148 insertions(+)

In file included from libavcodec/vaapi_encode.c:27:0:
libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’
 VAEncROI   *roi;
 ^
make: *** [libavcodec/vaapi_encode.o] Error 1
make: *** Waiting for unfinished jobs
In file included from libavcodec/vaapi_encode_mpeg2.c:28:0:
libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’
 VAEncROI   *roi;
 ^
make: *** [libavcodec/vaapi_encode_mpeg2.o] Error 1
POD doc/ffprobe.pod
In file included from libavcodec/vaapi_encode_h264.c:36:0:
libavcodec/vaapi_encode.h:73:5: error: unknown type name ‘VAEncROI’
 VAEncROI   *roi;
 ^
make: *** [libavcodec/vaapi_encode_h264.o] Error 1



[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship: All citizens are under surveillance, all their steps and
actions recorded, for the politicians to enforce control.
Democracy: All politicians are under surveillance, all their steps and
actions recorded, for the citizens to enforce control.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


[FFmpeg-devel] [PATCH] Don't trigger errors for multiple id3 tags.

2019-02-28 Thread Dale Curtis
Such errors may make sense for specific formats, but general parsing
logic shouldn't be treating these as errors regardless of the error
recognition mode.

Fixes loading of the following wave when using -err_detect explode:
https://cs.chromium.org/chromium/src/third_party/blink/web_tests/external/wpt/webaudio/resources/4ch-440.wav

Signed-off-by: Dale Curtis 
From 14c2244631e7d02d6f66a6d5a678125002b89675 Mon Sep 17 00:00:00 2001
From: Dale Curtis 
Date: Thu, 28 Feb 2019 13:51:30 -0800
Subject: [PATCH] Don't trigger errors for multiple id3 tags.

Such errors may make sense for specific formats, but general parsing
logic shouldn't be treating these as errors regardless of the error
recognition mode.

Fixes loading of the following wave when using -err_detect explode:
https://cs.chromium.org/chromium/src/third_party/blink/web_tests/external/wpt/webaudio/resources/4ch-440.wav

Signed-off-by: Dale Curtis 
---
 libavformat/utils.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/libavformat/utils.c b/libavformat/utils.c
index d113a16c80..b940246512 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -635,13 +635,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
 s->metadata = s->internal->id3v2_meta;
 s->internal->id3v2_meta = NULL;
 } else if (s->internal->id3v2_meta) {
-int level = AV_LOG_WARNING;
-if (s->error_recognition & AV_EF_COMPLIANT)
-level = AV_LOG_ERROR;
-av_log(s, level, "Discarding ID3 tags because more suitable tags were found.\n");
+av_log(s, AV_LOG_WARNING, "Discarding ID3 tags because more suitable tags were found.\n");
 av_dict_free(>internal->id3v2_meta);
-if (s->error_recognition & AV_EF_EXPLODE)
-return AVERROR_INVALIDDATA;
 }
 
 if (id3v2_extra_meta) {
-- 
2.21.0.352.gf09ad66450-goog

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


Re: [FFmpeg-devel] The log level of "co located POCs unavailable" should not be ERROR

2019-02-28 Thread Michael Niedermayer
On Thu, Feb 28, 2019 at 11:33:50AM +0800, Yukun Guo wrote:
> The error message "co located POCs unavailable" is reported when the
> co-located picture of an H.264 B frame is missing during spatial direct
> predition. It comes from this line:
> https://github.com/FFmpeg/FFmpeg/blob/n4.1.1/libavcodec/h264_direct.c#L156
> and was originally commited for solving an issue found by fuzzing:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1230286.
> 
> I think the condition is quite normal for live streaming, 1) when the
> player first connects to the streaming server, it may receive B frames
> whose reference frames have never been sent; 2) when the bandwidth is
> limited, the server purposely drops packets due to buffer overflow.
> IMHO the log level should be on par with "Frame num gap" (if
> `gaps_in_frame_num_value_allowed_flag` is false) and "no picture"
> (both are found in h264_slice.c and use DEBUG level).

/**
 * Something went wrong and cannot losslessly be recovered.
 * However, not all future data is affected.
 */
#define AV_LOG_ERROR16

When data is missing (in live streaming or otherwise) it under almost all
cases cannot be losslessly recovered. So this use matches the definition

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] GSoC 2019

2019-02-28 Thread Tomas Härdin
tis 2019-02-26 klockan 18:24 +0100 skrev Thilo Borgmann:
> Hi,
> 
> FFmpeg has just been selected as a mentoring organization for Google
> Summer of Code 2019 [1]!
> 
> As usual, we are still looking for more interesting student projects
> and mentors from our community willing to mentor.
> So please feel free to suggest a project and volunteer as mentor or
> backup mentor - you're welcome to edit the wiki directly or post your
> suggestion here on the list.
> 
> Find our wiki page here:
> https://trac.ffmpeg.org/wiki/SponsoringPrograms/GSoC/2019
> 
> Any feedback and help is appreciated!

Since AVIF seems to be nearing finalization, maybe adding support for
it could be a good project?

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


Re: [FFmpeg-devel] [PATCH] avformat/westwood_aud: Adds PCM format demux.

2019-02-28 Thread Tomas Härdin
ons 2019-02-27 klockan 23:12 + skrev Aidan R:
> PCM format AUD files are found in Westwood's Blade Runner game.

Do we have samples for this?

> @@ -100,6 +119,15 @@ static int wsaud_read_header(AVFormatContext *s)
>  return AVERROR(ENOMEM);
>  
>  switch (codec) {
> +case  0:
> +if (bits_per_sample == 8) {
> +st->codecpar->codec_id = AV_CODEC_ID_PCM_U8;
> +} else {
> +st->codecpar->codec_id = AV_CODEC_ID_PCM_S16LE;
> +}
> +st->codecpar->bits_per_coded_sample = bits_per_sample;
> +st->codecpar->bit_rate = channels * sample_rate * bits_per_sample;
> +break;
>  case  1:
>  if (channels != 1) {
>  avpriv_request_sample(s, "Stereo WS-SND1");
> @@ -130,20 +158,24 @@ static int wsaud_read_packet(AVFormatContext *s,
>   AVPacket *pkt)
>  {
>  AVIOContext *pb = s->pb;
> +AUDDemuxContext *aud = s->priv_data;
>  unsigned char preamble[AUD_CHUNK_PREAMBLE_SIZE];
> -unsigned int chunk_size;
> +unsigned int chunk_size, bytes_per_sample;
>  int ret = 0;
>  AVStream *st = s->streams[0];
>  
> -if (avio_read(pb, preamble, AUD_CHUNK_PREAMBLE_SIZE) !=
> -AUD_CHUNK_PREAMBLE_SIZE)
> -return AVERROR(EIO);
> +/* AUD files don't store PCM audio in chunks */
> +if (st->codecpar->codec_id != AV_CODEC_ID_PCM_S16LE) {

what about u8?

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


[FFmpeg-devel] [PATCH] Parallelize vf_lut

2019-02-28 Thread Britt Cyr
This will use ff_filter_get_nb_threads(ctx) threads which was 4x
faster for when I was testing on a 4K video
---
 libavfilter/vf_lut.c | 106 ---
 1 file changed, 70 insertions(+), 36 deletions(-)

diff --git a/libavfilter/vf_lut.c b/libavfilter/vf_lut.c
index c815ddc194..9e5527e4a1 100644
--- a/libavfilter/vf_lut.c
+++ b/libavfilter/vf_lut.c
@@ -72,6 +72,12 @@ typedef struct LutContext {
 int negate_alpha; /* only used by negate */
 } LutContext;
 
+typedef struct ThreadData {
+AVFrame *in;
+AVFrame *out;
+AVFilterLink *link;
+} ThreadData;
+
 #define Y 0
 #define U 1
 #define V 2
@@ -337,26 +343,13 @@ static int config_props(AVFilterLink *inlink)
 return 0;
 }
 
-static int filter_frame(AVFilterLink *inlink, AVFrame *in)
-{
-AVFilterContext *ctx = inlink->dst;
+static int lookup_slice(AVFilterContext *ctx, void *arg, int jobnr, int 
nb_jobs) {
 LutContext *s = ctx->priv;
-AVFilterLink *outlink = ctx->outputs[0];
-AVFrame *out;
-int i, j, plane, direct = 0;
-
-if (av_frame_is_writable(in)) {
-direct = 1;
-out = in;
-} else {
-out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
-if (!out) {
-av_frame_free();
-return AVERROR(ENOMEM);
-}
-av_frame_copy_props(out, in);
-}
-
+int i, j, plane = 0;
+const ThreadData *td = arg;
+const AVFrame *in  = td->in;
+AVFrame *out = td->out;
+const AVFilterLink *inlink = td->link;
 if (s->is_rgb && s->is_16bit && !s->is_planar) {
 /* packed, 16-bit */
 uint16_t *inrow, *outrow, *inrow0, *outrow0;
@@ -366,11 +359,13 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 const int in_linesize  =  in->linesize[0] / 2;
 const int out_linesize = out->linesize[0] / 2;
 const int step = s->step;
+const int row_min = jobnr / nb_jobs * h;
+const int row_max = (jobnr + 1) / nb_jobs * h;
 
 inrow0  = (uint16_t*) in ->data[0];
 outrow0 = (uint16_t*) out->data[0];
 
-for (i = 0; i < h; i ++) {
+for (i = row_min; i < row_max; i ++) {
 inrow  = inrow0;
 outrow = outrow0;
 for (j = 0; j < w; j++) {
@@ -403,11 +398,13 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 const int in_linesize  =  in->linesize[0];
 const int out_linesize = out->linesize[0];
 const int step = s->step;
+const int row_min = jobnr / nb_jobs * h;
+const int row_max = (jobnr + 1) / nb_jobs * h;
 
 inrow0  = in ->data[0];
 outrow0 = out->data[0];
 
-for (i = 0; i < h; i ++) {
+for (i = row_min; i < row_max; i ++) {
 inrow  = inrow0;
 outrow = outrow0;
 for (j = 0; j < w; j++) {
@@ -435,11 +432,13 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 const uint16_t *tab = s->lut[plane];
 const int in_linesize  =  in->linesize[plane] / 2;
 const int out_linesize = out->linesize[plane] / 2;
+const int row_min = jobnr / nb_jobs * h;
+const int row_max = (jobnr + 1) / nb_jobs * h;
 
 inrow  = (uint16_t *)in ->data[plane];
 outrow = (uint16_t *)out->data[plane];
 
-for (i = 0; i < h; i++) {
+for (i = row_min; i < row_max; i++) {
 for (j = 0; j < w; j++) {
 #if HAVE_BIGENDIAN
 outrow[j] = av_bswap16(tab[av_bswap16(inrow[j])]);
@@ -463,11 +462,13 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 const uint16_t *tab = s->lut[plane];
 const int in_linesize  =  in->linesize[plane];
 const int out_linesize = out->linesize[plane];
+const int row_min = jobnr / nb_jobs * h;
+const int row_max = (jobnr + 1) / nb_jobs * h;
 
 inrow  = in ->data[plane];
 outrow = out->data[plane];
 
-for (i = 0; i < h; i++) {
+for (i = row_min; i < row_max; i++) {
 for (j = 0; j < w; j++)
 outrow[j] = tab[inrow[j]];
 inrow  += in_linesize;
@@ -476,9 +477,42 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 }
 }
 
-if (!direct)
+return 0;
+}
+
+static AVFrame *apply_lut(AVFilterLink *inlink, AVFrame *in) {
+AVFilterContext *ctx = inlink->dst;
+AVFilterLink *outlink = ctx->outputs[0];
+AVFrame *out;
+ThreadData td;
+
+if (av_frame_is_writable(in)) {
+out = in;
+} else {
+out = ff_get_video_buffer(outlink, outlink->w, outlink->h);
+if (!out) {
+av_frame_free();
+return NULL;
+}
+av_frame_copy_props(out, in);
+}
+td.in  = in;
+td.out = out;
+td.link = inlink;
+ctx->internal->execute(ctx, lookup_slice, , NULL, FFMIN(outlink->h, 

Re: [FFmpeg-devel] [PATCH 5/5] lavfi: addroi filter

2019-02-28 Thread Nicolas George
Marton Balint (12019-02-28):
> Please use the activate callback of AVFilter context instead.

If it is one in, one out, it is not really necessary.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 5/5] lavfi: addroi filter

2019-02-28 Thread Marton Balint



On Wed, 27 Feb 2019, Mark Thompson wrote:


This can be used to add region of interest side data to video frames.
---
libavfilter/Makefile |   1 +
libavfilter/allfilters.c |   1 +
libavfilter/vf_addroi.c  | 237 +++
3 files changed, 239 insertions(+)
create mode 100644 libavfilter/vf_addroi.c


[...]


+static const AVFilterPad addroi_inputs[] = {
+{
+.name = "default",
+.type = AVMEDIA_TYPE_VIDEO,
+.config_props = addroi_config_input,
+.filter_frame = addroi_filter_frame,


Please use the activate callback of AVFilter context instead.

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


Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

2019-02-28 Thread James Almer
On 2/26/2019 10:18 PM, Chris Cunningham wrote:
> On Thu, Feb 21, 2019 at 4:46 PM Chris Cunningham
> mailto:chcunning...@chromium.org>> wrote:
> 
> I'm fine to do either. James, do you still prefer to skip the later
> headers if this breaks some old ogm files?
> 
> 
> James, friendly ping 

Yes, i'd prefer if we skip any superfluous parameter header that shows
up before the first data packet.
The return value of ff_codec_get_id() should also be checked to not
propagate AV_CODEC_ID_NONE, which was the source of this issue.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH 5/5] lavfi: addroi filter

2019-02-28 Thread Moritz Barsnick
On Thu, Feb 28, 2019 at 01:18:06 +, Mark Thompson wrote:
> > Can ROIs not also be defined as x, y, w, h?
> 
> Sure, it would be easy to change if people prefer that.

It was just my personal observation (and preference)

> This was chosen pretty much entirely because it matches the structure
> definition

Oh yeah, I thought so.

> Would "iw"/"ih" be better, or would that just be more confusing?

I'm not sure, I was just thinking consistence with other filters. You
do have an "outgoing" size, that's the size of the ROI. So perhaps
iw/ih also applies.

Just my €0,02,
Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2019-02-28 Thread Guo, Yejun
Signed-off-by: Guo, Yejun 
---
 libavcodec/libvpxenc.c | 131 +
 1 file changed, 131 insertions(+)

diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index c823b8a..0f8efcc 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -1057,6 +1057,134 @@ 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
+
+const AVRegionOfInterest *roi = NULL;
+vpx_roi_map_t roi_map;
+int ret = 0;
+int nb_rois;
+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 = 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};
+
+/* 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_mapping[MAX_DELTA_Q] = segment_id + 1;
+segment_id++;
+
+active_map.rows = (frame->height + 15) / 16;
+active_map.cols = (frame->width  + 15) / 16;
+
+if (!sd) {
+/* For the case that some middle frames do not have ROI while other 
frames have ROIs.
+ * Due to libvpx behaivor, we have to reset VP8E_SET_ACTIVEMAP, 
otherwise the previous
+ * ROIs continue working for frames without ROIs.
+ */
+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;
+}
+
+memset(_map, 0, sizeof(roi_map));
+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_log(avctx, AV_LOG_ERROR, "roi_map alloc failed.\n");
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+
+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 failed.\n");
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+/* set 1 to enable the corresponding element of vpx_roi_map_t.roi_map. */
+memset(active_map.active_map, 1, active_map.rows * active_map.cols);
+
+roi = (const AVRegionOfInterest*)sd->data;
+if (!roi->self_size || sd->size % roi->self_size != 0) {
+av_log(ctx, AV_LOG_ERROR, "Invalid AVRegionOfInterest.self_size.\n");
+ret = AVERROR(EINVAL);
+goto fail;
+}
+nb_rois = sd->size / roi->self_size;
+
+// This list must be iterated in reverse because regions are
+// defined in order of decreasing importance.
+for (int i = nb_rois - 1; i >= 0; i--) {
+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);
+
+roi = (const AVRegionOfInterest*)(sd->data + roi->self_size * i);
+if (roi->qoffset.den == 0) {
+av_log(ctx, AV_LOG_ERROR, "AVRegionOfInterest.qoffset.den must not 
be zero.\n");
+ret = AVERROR(EINVAL);
+goto fail;
+}
+
+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;
+}
+
+if (vpx_codec_control(>encoder, VP8E_SET_ROI_MAP, _map)) {
+log_encoder_error(avctx, "Failed to set VP8E_SET_ROI_MAP codec 
control.\n");
+ret = 

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 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
> 
>