Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Matthieu Bouron]

On Thu, Nov 26, 2015 at 01:12:44AM +0100, Michael Niedermayer wrote:
> On Wed, Nov 25, 2015 at 09:14:48PM +0100, Matthieu Bouron wrote:
> > On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> > > On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > > > From: Matthieu Bouron 
> [...]
> > >
> > > > +
> > > > +static int try_decode_video_frame(AVCodecContext *codec_ctx, AVPacket 
> > > > *pkt, int decode)
> > > > +{
> > > > +int ret = 0;
> > > > +int got_frame = 0;
> > > > +AVFrame *frame = NULL;
> > > > +int skip_frame = codec_ctx->skip_frame;
> > > > +
> > > > +if (!avcodec_is_open(codec_ctx)) {
> > > > +const AVCodec *codec = 
> > > > avcodec_find_decoder(codec_ctx->codec_id);
> > > > +
> > > > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > > > +if (ret < 0) {
> > > > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open codec\n");
> > > > +goto end;
> > > > +}
> > > > +}
> > > > +
> > > > +frame = av_frame_alloc();
> > > > +if (!frame) {
> > > > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > > > +goto end;
> > > > +}
> > > > +
> > > > +if (!decode && codec_ctx->codec->caps_internal & 
> > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > > > +}
> > > > +
> > > > +do {
> > > > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, pkt);
> > > > +av_assert0(decode || (!decode && !got_frame));
> > > > +if (ret < 0)
> > > > +break;
> > > > +pkt->data += ret;
> > > > +pkt->size -= ret;
> > > > +
> > > > +if (got_frame) {
> > > > +break;
> > > > +}
> > > > +} while (pkt->size > 0);
> > > > +
> > > > +end:
> > > > +codec_ctx->skip_frame = skip_frame;
> > > > +
> > > > +av_frame_free();
> > > > +return ret;
> > > > +}
> > > > +
> > > > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int decode)
> > > > +{
> > > > +int ret = 0;
> > > > +int i, done = 0;
> > > > +AVPacket pkt;
> > > > +
> > > > +av_init_packet();
> > > > +
> > > > +while (!done) {
> > > > +AVCodecContext *codec_ctx = NULL;
> > > > +AVStream *st;
> > > > +
> > > > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
> > > > +goto end;
> > > > +}
> > > > +
> > > > +st = fmt_ctx->streams[pkt.stream_index];
> > > > +codec_ctx = st->codec;
> > > > +
> > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> > > 
> > > > +st->codec_info_nb_frames++ > 0) {
> > > 
> > > writing into this is not ok, iam not sure it should be touched at
> > > all from API users and tests/api/* could be used as example for
> > > correct API use ...
> > 
> > This function intends to mimic avformat_find_stream_info which increments
> > this field, if it's a blocker i'll use something else.
> 
> presonally iam fine if you document that this should not be used by
> user applications and why it is used in this "regression test"
> 
> 
> > 
> > > 
> > > 
> > > > +av_packet_unref();
> > > > +continue;
> > > > +}
> > > > +
> > > > +ret = try_decode_video_frame(codec_ctx, , decode);
> > > > +if (ret < 0) {
> > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video 
> > > > frame\n");
> > > > +goto end;
> > > > +}
> > > > +
> > > > +av_packet_unref();
> > > > +
> > > > +/* check if all video streams have demuxed a packet */
> > > > +done = 1;
> > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > +st = fmt_ctx->streams[i];
> > > > +codec_ctx = st->codec;
> > > > +
> > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
> > > > +continue;
> > > > +
> > > > +done &= st->codec_info_nb_frames > 0;
> > > > +}
> > > > +}
> > > > +
> > > > +end:
> > > > +av_packet_unref();
> > > > +
> > > > +return ret < 0;
> > > > +}
> > > > +
> > > > +static void dump_video_streams(const AVFormatContext *fmt_ctx, int 
> > > > decode)
> > > > +{
> > > > +int i;
> > > > +
> > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > +const AVStream *st = fmt_ctx->streams[i];
> > > > +const AVCodecContext *codec_ctx = st->codec;
> > > > +const AVRational sar = codec_ctx->sample_aspect_ratio;
> > > > +
> > > > +printf("stream=%d, decode=%d\n", i, decode);
> > > > +printf("width=%d\n", codec_ctx->width);
> > > > +printf("height=%d\n", codec_ctx->height);
> > > > +printf("pix_fmt=%s\n", 
> > > > av_get_pix_fmt_name(codec_ctx->pix_fmt));
> > > > +printf("sar=%d/%d\n", sar.num, sar.den);
> > > > +  

Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Michael Niedermayer]

On Thu, Nov 26, 2015 at 11:57:46AM +0100, Matthieu Bouron wrote:
> On Thu, Nov 26, 2015 at 01:12:44AM +0100, Michael Niedermayer wrote:
> > On Wed, Nov 25, 2015 at 09:14:48PM +0100, Matthieu Bouron wrote:
> > > On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> > > > On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > > > > From: Matthieu Bouron 
> > [...]
> > > >
> > > > > +
> > > > > +static int try_decode_video_frame(AVCodecContext *codec_ctx, 
> > > > > AVPacket *pkt, int decode)
> > > > > +{
> > > > > +int ret = 0;
> > > > > +int got_frame = 0;
> > > > > +AVFrame *frame = NULL;
> > > > > +int skip_frame = codec_ctx->skip_frame;
> > > > > +
> > > > > +if (!avcodec_is_open(codec_ctx)) {
> > > > > +const AVCodec *codec = 
> > > > > avcodec_find_decoder(codec_ctx->codec_id);
> > > > > +
> > > > > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > > > > +if (ret < 0) {
> > > > > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open 
> > > > > codec\n");
> > > > > +goto end;
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +frame = av_frame_alloc();
> > > > > +if (!frame) {
> > > > > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > > > > +goto end;
> > > > > +}
> > > > > +
> > > > > +if (!decode && codec_ctx->codec->caps_internal & 
> > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > > > > +}
> > > > > +
> > > > > +do {
> > > > > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, 
> > > > > pkt);
> > > > > +av_assert0(decode || (!decode && !got_frame));
> > > > > +if (ret < 0)
> > > > > +break;
> > > > > +pkt->data += ret;
> > > > > +pkt->size -= ret;
> > > > > +
> > > > > +if (got_frame) {
> > > > > +break;
> > > > > +}
> > > > > +} while (pkt->size > 0);
> > > > > +
> > > > > +end:
> > > > > +codec_ctx->skip_frame = skip_frame;
> > > > > +
> > > > > +av_frame_free();
> > > > > +return ret;
> > > > > +}
> > > > > +
> > > > > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int 
> > > > > decode)
> > > > > +{
> > > > > +int ret = 0;
> > > > > +int i, done = 0;
> > > > > +AVPacket pkt;
> > > > > +
> > > > > +av_init_packet();
> > > > > +
> > > > > +while (!done) {
> > > > > +AVCodecContext *codec_ctx = NULL;
> > > > > +AVStream *st;
> > > > > +
> > > > > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
> > > > > +goto end;
> > > > > +}
> > > > > +
> > > > > +st = fmt_ctx->streams[pkt.stream_index];
> > > > > +codec_ctx = st->codec;
> > > > > +
> > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> > > > 
> > > > > +st->codec_info_nb_frames++ > 0) {
> > > > 
> > > > writing into this is not ok, iam not sure it should be touched at
> > > > all from API users and tests/api/* could be used as example for
> > > > correct API use ...
> > > 
> > > This function intends to mimic avformat_find_stream_info which increments
> > > this field, if it's a blocker i'll use something else.
> > 
> > presonally iam fine if you document that this should not be used by
> > user applications and why it is used in this "regression test"
> > 
> > 
> > > 
> > > > 
> > > > 
> > > > > +av_packet_unref();
> > > > > +continue;
> > > > > +}
> > > > > +
> > > > > +ret = try_decode_video_frame(codec_ctx, , decode);
> > > > > +if (ret < 0) {
> > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video 
> > > > > frame\n");
> > > > > +goto end;
> > > > > +}
> > > > > +
> > > > > +av_packet_unref();
> > > > > +
> > > > > +/* check if all video streams have demuxed a packet */
> > > > > +done = 1;
> > > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > > +st = fmt_ctx->streams[i];
> > > > > +codec_ctx = st->codec;
> > > > > +
> > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
> > > > > +continue;
> > > > > +
> > > > > +done &= st->codec_info_nb_frames > 0;
> > > > > +}
> > > > > +}
> > > > > +
> > > > > +end:
> > > > > +av_packet_unref();
> > > > > +
> > > > > +return ret < 0;
> > > > > +}
> > > > > +
> > > > > +static void dump_video_streams(const AVFormatContext *fmt_ctx, int 
> > > > > decode)
> > > > > +{
> > > > > +int i;
> > > > > +
> > > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > > +const AVStream *st = fmt_ctx->streams[i];
> > > > > +const AVCodecContext *codec_ctx = st->codec;
> > > > > +const AVRational sar = 

Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Matthieu Bouron]

On Thu, Nov 26, 2015 at 04:07:00PM +0100, Michael Niedermayer wrote:
> On Thu, Nov 26, 2015 at 11:57:46AM +0100, Matthieu Bouron wrote:
> > On Thu, Nov 26, 2015 at 01:12:44AM +0100, Michael Niedermayer wrote:
> > > On Wed, Nov 25, 2015 at 09:14:48PM +0100, Matthieu Bouron wrote:
> > > > On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> > > > > On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > > > > > From: Matthieu Bouron 
> > > [...]
> > > > >
> > > > > > +
> > > > > > +static int try_decode_video_frame(AVCodecContext *codec_ctx, 
> > > > > > AVPacket *pkt, int decode)
> > > > > > +{
> > > > > > +int ret = 0;
> > > > > > +int got_frame = 0;
> > > > > > +AVFrame *frame = NULL;
> > > > > > +int skip_frame = codec_ctx->skip_frame;
> > > > > > +
> > > > > > +if (!avcodec_is_open(codec_ctx)) {
> > > > > > +const AVCodec *codec = 
> > > > > > avcodec_find_decoder(codec_ctx->codec_id);
> > > > > > +
> > > > > > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > > > > > +if (ret < 0) {
> > > > > > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open 
> > > > > > codec\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +}
> > > > > > +
> > > > > > +frame = av_frame_alloc();
> > > > > > +if (!frame) {
> > > > > > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +
> > > > > > +if (!decode && codec_ctx->codec->caps_internal & 
> > > > > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > > > > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > > > > > +}
> > > > > > +
> > > > > > +do {
> > > > > > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, 
> > > > > > pkt);
> > > > > > +av_assert0(decode || (!decode && !got_frame));
> > > > > > +if (ret < 0)
> > > > > > +break;
> > > > > > +pkt->data += ret;
> > > > > > +pkt->size -= ret;
> > > > > > +
> > > > > > +if (got_frame) {
> > > > > > +break;
> > > > > > +}
> > > > > > +} while (pkt->size > 0);
> > > > > > +
> > > > > > +end:
> > > > > > +codec_ctx->skip_frame = skip_frame;
> > > > > > +
> > > > > > +av_frame_free();
> > > > > > +return ret;
> > > > > > +}
> > > > > > +
> > > > > > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int 
> > > > > > decode)
> > > > > > +{
> > > > > > +int ret = 0;
> > > > > > +int i, done = 0;
> > > > > > +AVPacket pkt;
> > > > > > +
> > > > > > +av_init_packet();
> > > > > > +
> > > > > > +while (!done) {
> > > > > > +AVCodecContext *codec_ctx = NULL;
> > > > > > +AVStream *st;
> > > > > > +
> > > > > > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read 
> > > > > > frame\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +
> > > > > > +st = fmt_ctx->streams[pkt.stream_index];
> > > > > > +codec_ctx = st->codec;
> > > > > > +
> > > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> > > > > 
> > > > > > +st->codec_info_nb_frames++ > 0) {
> > > > > 
> > > > > writing into this is not ok, iam not sure it should be touched at
> > > > > all from API users and tests/api/* could be used as example for
> > > > > correct API use ...
> > > > 
> > > > This function intends to mimic avformat_find_stream_info which 
> > > > increments
> > > > this field, if it's a blocker i'll use something else.
> > > 
> > > presonally iam fine if you document that this should not be used by
> > > user applications and why it is used in this "regression test"
> > > 
> > > 
> > > > 
> > > > > 
> > > > > 
> > > > > > +av_packet_unref();
> > > > > > +continue;
> > > > > > +}
> > > > > > +
> > > > > > +ret = try_decode_video_frame(codec_ctx, , decode);
> > > > > > +if (ret < 0) {
> > > > > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video 
> > > > > > frame\n");
> > > > > > +goto end;
> > > > > > +}
> > > > > > +
> > > > > > +av_packet_unref();
> > > > > > +
> > > > > > +/* check if all video streams have demuxed a packet */
> > > > > > +done = 1;
> > > > > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > > > > +st = fmt_ctx->streams[i];
> > > > > > +codec_ctx = st->codec;
> > > > > > +
> > > > > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
> > > > > > +continue;
> > > > > > +
> > > > > > +done &= st->codec_info_nb_frames > 0;
> > > > > > +}
> > > > > > +}
> > > > > > +
> > > > > > +end:
> > > > > > +av_packet_unref();
> > > > > > +
> > > > > > +return ret < 0;
> > > > > > +}
> > > > > > +
> > > > > > +static void 

Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Michael Niedermayer]

On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> From: Matthieu Bouron 
> 
> ---
>  tests/api/Makefile  |   1 +
>  tests/api/api-codec-param-test.c| 223 
> 
>  tests/fate/api.mak  |   8 ++
>  tests/ref/fate/api-jpeg-codec-param |  12 ++
>  tests/ref/fate/api-png-codec-param  |  12 ++
>  5 files changed, 256 insertions(+)
>  create mode 100644 tests/api/api-codec-param-test.c
>  create mode 100644 tests/ref/fate/api-jpeg-codec-param
>  create mode 100644 tests/ref/fate/api-png-codec-param
> 
> diff --git a/tests/api/Makefile b/tests/api/Makefile
> index 27f499f..c48c34a 100644
> --- a/tests/api/Makefile
> +++ b/tests/api/Makefile
> @@ -1,6 +1,7 @@
>  APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac
>  APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264
>  APITESTPROGS-yes += api-seek
> +APITESTPROGS-yes += api-codec-param
>  APITESTPROGS-$(call DEMDEC, H263, H263) += api-band
>  APITESTPROGS += $(APITESTPROGS-yes)
>  
> diff --git a/tests/api/api-codec-param-test.c 
> b/tests/api/api-codec-param-test.c
> new file mode 100644
> index 000..48aced9
> --- /dev/null
> +++ b/tests/api/api-codec-param-test.c
> @@ -0,0 +1,223 @@
> +/*
> + * Copyright (c) 2015 Matthieu Bouron 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

did you intend to use #include <> instead of "" ?


> +
> +static int try_decode_video_frame(AVCodecContext *codec_ctx, AVPacket *pkt, 
> int decode)
> +{
> +int ret = 0;
> +int got_frame = 0;
> +AVFrame *frame = NULL;
> +int skip_frame = codec_ctx->skip_frame;
> +
> +if (!avcodec_is_open(codec_ctx)) {
> +const AVCodec *codec = avcodec_find_decoder(codec_ctx->codec_id);
> +
> +ret = avcodec_open2(codec_ctx, codec, NULL);
> +if (ret < 0) {
> +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open codec\n");
> +goto end;
> +}
> +}
> +
> +frame = av_frame_alloc();
> +if (!frame) {
> +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> +goto end;
> +}
> +
> +if (!decode && codec_ctx->codec->caps_internal & 
> FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> +codec_ctx->skip_frame = AVDISCARD_ALL;
> +}
> +
> +do {
> +ret = avcodec_decode_video2(codec_ctx, frame, _frame, pkt);
> +av_assert0(decode || (!decode && !got_frame));
> +if (ret < 0)
> +break;
> +pkt->data += ret;
> +pkt->size -= ret;
> +
> +if (got_frame) {
> +break;
> +}
> +} while (pkt->size > 0);
> +
> +end:
> +codec_ctx->skip_frame = skip_frame;
> +
> +av_frame_free();
> +return ret;
> +}
> +
> +static int find_video_stream_info(AVFormatContext *fmt_ctx, int decode)
> +{
> +int ret = 0;
> +int i, done = 0;
> +AVPacket pkt;
> +
> +av_init_packet();
> +
> +while (!done) {
> +AVCodecContext *codec_ctx = NULL;
> +AVStream *st;
> +
> +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
> +goto end;
> +}
> +
> +st = fmt_ctx->streams[pkt.stream_index];
> +codec_ctx = st->codec;
> +
> +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||

> +st->codec_info_nb_frames++ > 0) {

writing into this is not ok, iam not sure it should be touched at
all from API users and tests/api/* could be used as example for
correct API use ...


> +av_packet_unref();
> +continue;
> +}
> +
> +ret = try_decode_video_frame(codec_ctx, , decode);
> +if (ret < 0) {
> +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video frame\n");
> +goto end;
> +}
> 

Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Michael Niedermayer]

On Wed, Nov 25, 2015 at 09:14:48PM +0100, Matthieu Bouron wrote:
> On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> > On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > > From: Matthieu Bouron 
[...]
> >
> > > +
> > > +static int try_decode_video_frame(AVCodecContext *codec_ctx, AVPacket 
> > > *pkt, int decode)
> > > +{
> > > +int ret = 0;
> > > +int got_frame = 0;
> > > +AVFrame *frame = NULL;
> > > +int skip_frame = codec_ctx->skip_frame;
> > > +
> > > +if (!avcodec_is_open(codec_ctx)) {
> > > +const AVCodec *codec = avcodec_find_decoder(codec_ctx->codec_id);
> > > +
> > > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > > +if (ret < 0) {
> > > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open codec\n");
> > > +goto end;
> > > +}
> > > +}
> > > +
> > > +frame = av_frame_alloc();
> > > +if (!frame) {
> > > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > > +goto end;
> > > +}
> > > +
> > > +if (!decode && codec_ctx->codec->caps_internal & 
> > > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > > +}
> > > +
> > > +do {
> > > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, pkt);
> > > +av_assert0(decode || (!decode && !got_frame));
> > > +if (ret < 0)
> > > +break;
> > > +pkt->data += ret;
> > > +pkt->size -= ret;
> > > +
> > > +if (got_frame) {
> > > +break;
> > > +}
> > > +} while (pkt->size > 0);
> > > +
> > > +end:
> > > +codec_ctx->skip_frame = skip_frame;
> > > +
> > > +av_frame_free();
> > > +return ret;
> > > +}
> > > +
> > > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int decode)
> > > +{
> > > +int ret = 0;
> > > +int i, done = 0;
> > > +AVPacket pkt;
> > > +
> > > +av_init_packet();
> > > +
> > > +while (!done) {
> > > +AVCodecContext *codec_ctx = NULL;
> > > +AVStream *st;
> > > +
> > > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
> > > +goto end;
> > > +}
> > > +
> > > +st = fmt_ctx->streams[pkt.stream_index];
> > > +codec_ctx = st->codec;
> > > +
> > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> > 
> > > +st->codec_info_nb_frames++ > 0) {
> > 
> > writing into this is not ok, iam not sure it should be touched at
> > all from API users and tests/api/* could be used as example for
> > correct API use ...
> 
> This function intends to mimic avformat_find_stream_info which increments
> this field, if it's a blocker i'll use something else.

presonally iam fine if you document that this should not be used by
user applications and why it is used in this "regression test"


> 
> > 
> > 
> > > +av_packet_unref();
> > > +continue;
> > > +}
> > > +
> > > +ret = try_decode_video_frame(codec_ctx, , decode);
> > > +if (ret < 0) {
> > > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video 
> > > frame\n");
> > > +goto end;
> > > +}
> > > +
> > > +av_packet_unref();
> > > +
> > > +/* check if all video streams have demuxed a packet */
> > > +done = 1;
> > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > +st = fmt_ctx->streams[i];
> > > +codec_ctx = st->codec;
> > > +
> > > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
> > > +continue;
> > > +
> > > +done &= st->codec_info_nb_frames > 0;
> > > +}
> > > +}
> > > +
> > > +end:
> > > +av_packet_unref();
> > > +
> > > +return ret < 0;
> > > +}
> > > +
> > > +static void dump_video_streams(const AVFormatContext *fmt_ctx, int 
> > > decode)
> > > +{
> > > +int i;
> > > +
> > > +for (i = 0; i < fmt_ctx->nb_streams; i++) {
> > > +const AVStream *st = fmt_ctx->streams[i];
> > > +const AVCodecContext *codec_ctx = st->codec;
> > > +const AVRational sar = codec_ctx->sample_aspect_ratio;
> > > +
> > > +printf("stream=%d, decode=%d\n", i, decode);
> > > +printf("width=%d\n", codec_ctx->width);
> > > +printf("height=%d\n", codec_ctx->height);
> > > +printf("pix_fmt=%s\n", 
> > > av_get_pix_fmt_name(codec_ctx->pix_fmt));
> > > +printf("sar=%d/%d\n", sar.num, sar.den);
> > > +printf("\n");
> > > +}
> > > +}
> > > +
> > > +static int open_and_probe_video_streams(AVFormatContext **fmt_ctx, const 
> > > char *filename, int decode)
> > > +{
> > > +int ret = 0;
> > > +
> > > +ret = avformat_open_input(fmt_ctx, filename, NULL, NULL);
> > > +if (ret < 0) {
> > > +av_log(NULL, AV_LOG_ERROR, "Failed to open input 

Re: [FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Matthieu Bouron]

On Wed, Nov 25, 2015 at 06:36:03PM +0100, Michael Niedermayer wrote:
> On Wed, Nov 25, 2015 at 03:40:15PM +0100, Matthieu Bouron wrote:
> > From: Matthieu Bouron 
> > 
> > ---
> >  tests/api/Makefile  |   1 +
> >  tests/api/api-codec-param-test.c| 223 
> > 
> >  tests/fate/api.mak  |   8 ++
> >  tests/ref/fate/api-jpeg-codec-param |  12 ++
> >  tests/ref/fate/api-png-codec-param  |  12 ++
> >  5 files changed, 256 insertions(+)
> >  create mode 100644 tests/api/api-codec-param-test.c
> >  create mode 100644 tests/ref/fate/api-jpeg-codec-param
> >  create mode 100644 tests/ref/fate/api-png-codec-param
> > 
> > diff --git a/tests/api/Makefile b/tests/api/Makefile
> > index 27f499f..c48c34a 100644
> > --- a/tests/api/Makefile
> > +++ b/tests/api/Makefile
> > @@ -1,6 +1,7 @@
> >  APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac
> >  APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264
> >  APITESTPROGS-yes += api-seek
> > +APITESTPROGS-yes += api-codec-param
> >  APITESTPROGS-$(call DEMDEC, H263, H263) += api-band
> >  APITESTPROGS += $(APITESTPROGS-yes)
> >  
> > diff --git a/tests/api/api-codec-param-test.c 
> > b/tests/api/api-codec-param-test.c
> > new file mode 100644
> > index 000..48aced9
> > --- /dev/null
> > +++ b/tests/api/api-codec-param-test.c
> > @@ -0,0 +1,223 @@
> > +/*
> > + * Copyright (c) 2015 Matthieu Bouron 
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> 
> did you intend to use #include <> instead of "" ?

Nope (fixed locally).

> 
> 
> > +
> > +static int try_decode_video_frame(AVCodecContext *codec_ctx, AVPacket 
> > *pkt, int decode)
> > +{
> > +int ret = 0;
> > +int got_frame = 0;
> > +AVFrame *frame = NULL;
> > +int skip_frame = codec_ctx->skip_frame;
> > +
> > +if (!avcodec_is_open(codec_ctx)) {
> > +const AVCodec *codec = avcodec_find_decoder(codec_ctx->codec_id);
> > +
> > +ret = avcodec_open2(codec_ctx, codec, NULL);
> > +if (ret < 0) {
> > +av_log(codec_ctx, AV_LOG_ERROR, "Failed to open codec\n");
> > +goto end;
> > +}
> > +}
> > +
> > +frame = av_frame_alloc();
> > +if (!frame) {
> > +av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
> > +goto end;
> > +}
> > +
> > +if (!decode && codec_ctx->codec->caps_internal & 
> > FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
> > +codec_ctx->skip_frame = AVDISCARD_ALL;
> > +}
> > +
> > +do {
> > +ret = avcodec_decode_video2(codec_ctx, frame, _frame, pkt);
> > +av_assert0(decode || (!decode && !got_frame));
> > +if (ret < 0)
> > +break;
> > +pkt->data += ret;
> > +pkt->size -= ret;
> > +
> > +if (got_frame) {
> > +break;
> > +}
> > +} while (pkt->size > 0);
> > +
> > +end:
> > +codec_ctx->skip_frame = skip_frame;
> > +
> > +av_frame_free();
> > +return ret;
> > +}
> > +
> > +static int find_video_stream_info(AVFormatContext *fmt_ctx, int decode)
> > +{
> > +int ret = 0;
> > +int i, done = 0;
> > +AVPacket pkt;
> > +
> > +av_init_packet();
> > +
> > +while (!done) {
> > +AVCodecContext *codec_ctx = NULL;
> > +AVStream *st;
> > +
> > +if ((ret = av_read_frame(fmt_ctx, )) < 0) {
> > +av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
> > +goto end;
> > +}
> > +
> > +st = fmt_ctx->streams[pkt.stream_index];
> > +codec_ctx = st->codec;
> > +
> > +if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
> 
> > +st->codec_info_nb_frames++ > 0) {
> 
> writing into this is not ok, 

[FFmpeg-devel] [PATCH] fate: add FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM tests

[Matthieu Bouron]

From: Matthieu Bouron 

---
 tests/api/Makefile  |   1 +
 tests/api/api-codec-param-test.c| 223 
 tests/fate/api.mak  |   8 ++
 tests/ref/fate/api-jpeg-codec-param |  12 ++
 tests/ref/fate/api-png-codec-param  |  12 ++
 5 files changed, 256 insertions(+)
 create mode 100644 tests/api/api-codec-param-test.c
 create mode 100644 tests/ref/fate/api-jpeg-codec-param
 create mode 100644 tests/ref/fate/api-png-codec-param

diff --git a/tests/api/Makefile b/tests/api/Makefile
index 27f499f..c48c34a 100644
--- a/tests/api/Makefile
+++ b/tests/api/Makefile
@@ -1,6 +1,7 @@
 APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac
 APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264
 APITESTPROGS-yes += api-seek
+APITESTPROGS-yes += api-codec-param
 APITESTPROGS-$(call DEMDEC, H263, H263) += api-band
 APITESTPROGS += $(APITESTPROGS-yes)
 
diff --git a/tests/api/api-codec-param-test.c b/tests/api/api-codec-param-test.c
new file mode 100644
index 000..48aced9
--- /dev/null
+++ b/tests/api/api-codec-param-test.c
@@ -0,0 +1,223 @@
+/*
+ * Copyright (c) 2015 Matthieu Bouron 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int try_decode_video_frame(AVCodecContext *codec_ctx, AVPacket *pkt, 
int decode)
+{
+int ret = 0;
+int got_frame = 0;
+AVFrame *frame = NULL;
+int skip_frame = codec_ctx->skip_frame;
+
+if (!avcodec_is_open(codec_ctx)) {
+const AVCodec *codec = avcodec_find_decoder(codec_ctx->codec_id);
+
+ret = avcodec_open2(codec_ctx, codec, NULL);
+if (ret < 0) {
+av_log(codec_ctx, AV_LOG_ERROR, "Failed to open codec\n");
+goto end;
+}
+}
+
+frame = av_frame_alloc();
+if (!frame) {
+av_log(NULL, AV_LOG_ERROR, "Failed to allocate frame\n");
+goto end;
+}
+
+if (!decode && codec_ctx->codec->caps_internal & 
FF_CODEC_CAP_SKIP_FRAME_FILL_PARAM) {
+codec_ctx->skip_frame = AVDISCARD_ALL;
+}
+
+do {
+ret = avcodec_decode_video2(codec_ctx, frame, _frame, pkt);
+av_assert0(decode || (!decode && !got_frame));
+if (ret < 0)
+break;
+pkt->data += ret;
+pkt->size -= ret;
+
+if (got_frame) {
+break;
+}
+} while (pkt->size > 0);
+
+end:
+codec_ctx->skip_frame = skip_frame;
+
+av_frame_free();
+return ret;
+}
+
+static int find_video_stream_info(AVFormatContext *fmt_ctx, int decode)
+{
+int ret = 0;
+int i, done = 0;
+AVPacket pkt;
+
+av_init_packet();
+
+while (!done) {
+AVCodecContext *codec_ctx = NULL;
+AVStream *st;
+
+if ((ret = av_read_frame(fmt_ctx, )) < 0) {
+av_log(fmt_ctx, AV_LOG_ERROR, "Failed to read frame\n");
+goto end;
+}
+
+st = fmt_ctx->streams[pkt.stream_index];
+codec_ctx = st->codec;
+
+if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO ||
+st->codec_info_nb_frames++ > 0) {
+av_packet_unref();
+continue;
+}
+
+ret = try_decode_video_frame(codec_ctx, , decode);
+if (ret < 0) {
+av_log(fmt_ctx, AV_LOG_ERROR, "Failed to decode video frame\n");
+goto end;
+}
+
+av_packet_unref();
+
+/* check if all video streams have demuxed a packet */
+done = 1;
+for (i = 0; i < fmt_ctx->nb_streams; i++) {
+st = fmt_ctx->streams[i];
+codec_ctx = st->codec;
+
+if (codec_ctx->codec_type != AVMEDIA_TYPE_VIDEO)
+continue;
+
+done &= st->codec_info_nb_frames > 0;
+}
+}
+
+end:
+av_packet_unref();
+
+return ret < 0;
+}
+
+static void dump_video_streams(const AVFormatContext *fmt_ctx, int decode)
+{
+int i;
+
+for