On 10/10/2013 01:48 PM, Vittorio Giovara wrote:
> On Thursday, October 10, 2013, John Stebbins wrote:
>
>> This can be optionally disabled whith the "showall" flags2 option.
>> When in "showall" mode, incomplete frames are signalled through
>> AVFrame.flags FRAME_FLAG_INCOMPLETE_FRAME.
>> ---
>> libavcodec/avcodec.h | 1 +
>> libavcodec/h264.c | 44
>> ++++++++++++++++++++++++++++++++++++++++----
>> libavcodec/h264.h | 9 +++++++++
>> libavcodec/mpegvideo.h | 1 +
>> libavcodec/options_table.h | 1 +
>> libavutil/frame.c | 1 +
>> libavutil/frame.h | 3 +++
>> 7 files changed, 56 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index d535308..c76680d 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -660,6 +660,7 @@ typedef struct RcOverride{
>> #define CODEC_FLAG2_IGNORE_CROP 0x00010000 ///< Discard cropping
>> information from SPS.
>>
>> #define CODEC_FLAG2_CHUNKS 0x00008000 ///< Input bitstream might
>> be truncated at a packet boundaries instead of only at frame boundaries.
>> +#define CODEC_FLAG2_SHOW_ALL 0x00400000 ///< Show all frames before
>> the first keyframe
>
> It'd nice if you bumped _MICRO version because of this flag.
Ok
>
>
>> /* Unsupported options :
>> * Syntax Arithmetic coding (SAC)
>> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
>> index 7311e6a..0024b33 100644
>> --- a/libavcodec/h264.c
>> +++ b/libavcodec/h264.c
>> @@ -336,6 +336,7 @@ static int ref_picture(H264Context *h, Picture *dst,
>> Picture *src)
>> dst->field_picture = src->field_picture;
>> dst->needs_realloc = src->needs_realloc;
>> dst->reference = src->reference;
>> + dst->recovered = src->recovered;
>>
>> return 0;
>> fail:
>> @@ -1560,6 +1561,8 @@ av_cold int ff_h264_decode_init(AVCodecContext
>> *avctx)
>> h->prev_poc_msb = 1 << 16;
>> h->x264_build = -1;
>> ff_h264_reset_sei(h);
>> + h->recovery_frame = -1;
>> + h->frame_recovered = 0;
>> if (avctx->codec_id == AV_CODEC_ID_H264) {
>> if (avctx->ticks_per_frame == 1)
>> h->avctx->time_base.den *= 2;
>> @@ -1830,6 +1833,9 @@ static int
>> decode_update_thread_context(AVCodecContext *dst,
>> h->prev_frame_num = h->frame_num;
>> h->outputed_poc = h->next_outputed_poc;
>>
>> + h->recovery_frame = h1->recovery_frame;
>> + h->frame_recovered = h1->frame_recovered;
>> +
>> return err;
>> }
>>
>> @@ -1859,6 +1865,7 @@ static int h264_frame_start(H264Context *h)
>> */
>> pic->f.key_frame = 0;
>> pic->mmco_reset = 0;
>> + pic->recovered = 0;
>>
>> if ((ret = alloc_picture(h, pic)) < 0)
>> return ret;
>> @@ -2130,6 +2137,15 @@ static void decode_postinit(H264Context *h, int
>> setup_finished)
>> av_log(h->avctx, AV_LOG_DEBUG, "no picture\n");
>> }
>>
>> + if (h->next_output_pic) {
>> + if (h->next_output_pic->recovered) {
>> + // Setting bit 0 means we have reached an recovery point and
>> all
>> + // frames after it in display order are "recovered".
>> + h->frame_recovered |= 1;
>> + }
>> + h->next_output_pic->recovered |= !!(h->frame_recovered & 1);
>> + }
>> +
>> if (setup_finished && !h->avctx->hwaccel)
>> ff_thread_finish_setup(h->avctx);
>> }
>> @@ -2711,6 +2727,8 @@ static void flush_change(H264Context *h)
>> memset(h->default_ref_list[0], 0, sizeof(h->default_ref_list[0]));
>> memset(h->default_ref_list[1], 0, sizeof(h->default_ref_list[1]));
>> ff_h264_reset_sei(h);
>> + h->recovery_frame = -1;
>> + h->frame_recovered = 0;
>> }
>>
>> /* forget old pics after a seek */
>> @@ -4587,10 +4605,26 @@ again:
>> if ((err = decode_slice_header(hx, h)))
>> break;
>>
>> + if (h->sei_recovery_frame_cnt >= 0 && h->recovery_frame <
>> 0) {
>> + h->recovery_frame =
>> + (h->frame_num + h->sei_recovery_frame_cnt) %
>> + (1 << h->sps.log2_max_frame_num);
>> + }
>> +
>> h->cur_pic_ptr->f.key_frame |=
>> (hx->nal_unit_type == NAL_IDR_SLICE) ||
>> (h->sei_recovery_frame_cnt >= 0);
>>
>> + if (hx->nal_unit_type == NAL_IDR_SLICE ||
>> + h->recovery_frame == h->frame_num) {
>> + h->recovery_frame = -1;
>> + h->cur_pic_ptr->recovered = 1;
>> + }
>> + // Setting bit 1 means we have seen an IDR and all frames
>> + // after it in decoded order are "recovered".
>> + h->frame_recovered |= 2 * !!(hx->nal_unit_type ==
>> NAL_IDR_SLICE);
>> + h->cur_pic_ptr->recovered |= !!(h->frame_recovered & 2);
>> +
>> if (h->current_slice == 1) {
>> if (!(avctx->flags2 & CODEC_FLAG2_CHUNKS))
>> decode_postinit(h, nal_index >= nals_needed);
>> @@ -4820,10 +4854,12 @@ out:
>>
>> field_end(h, 0);
>>
>> - if (!h->next_output_pic) {
>> - /* Wait for second field. */
>> - *got_frame = 0;
>> - } else {
>> + /* Wait for second field. */
>> + *got_frame = 0;
>> + if (h->next_output_pic && ((avctx->flags2 & CODEC_FLAG2_SHOW_ALL)
>> ||
>> + h->next_output_pic->recovered)) {
>> + if (!h->next_output_pic->recovered)
>> + h->next_output_pic->f.flags |=
>> FRAME_FLAG_INCOMPLETE_FRAME;
>> ret = output_frame(h, pict, &h->next_output_pic->f);
>> if (ret < 0)
>> return ret;
>> diff --git a/libavcodec/h264.h b/libavcodec/h264.h
>> index 3ef8420..0fe9921 100644
>> --- a/libavcodec/h264.h
>> +++ b/libavcodec/h264.h
>> @@ -611,6 +611,15 @@ typedef struct H264Context {
>> */
>> int sei_recovery_frame_cnt;
>>
>> + /**
>> + * recovery_frame is the frame_num at which the next frame should
>> + * be fully constructed.
>> + *
>> + * Set to -1 when not expecting a recovery point.
>> + */
>> + int recovery_frame;
>> + int frame_recovered; ///< Initial frame has been completely
>> recovered
>> +
>> int luma_weight_flag[2]; ///< 7.4.3.2 luma_weight_lX_flag
>> int chroma_weight_flag[2]; ///< 7.4.3.2 chroma_weight_lX_flag
>>
>> diff --git a/libavcodec/mpegvideo.h b/libavcodec/mpegvideo.h
>> index 81e3d2b..48b3cae 100644
>> --- a/libavcodec/mpegvideo.h
>> +++ b/libavcodec/mpegvideo.h
>> @@ -172,6 +172,7 @@ typedef struct Picture{
>>
>> int reference;
>> int shared;
>> + int recovered; ///< Picture at IDR or recovery point +
>> recovery count
>> } Picture;
>
> I'm not sure I understand why this is needed here, AVFrame seems enough to
> store recovery information no?
AVFrame is a public struct. Applications don't need to see this internal state
tracking data.
>
>
>> /**
>> diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
>> index 5e9d484..9113272 100644
>> --- a/libavcodec/options_table.h
>> +++ b/libavcodec/options_table.h
>> @@ -72,6 +72,7 @@ static const AVOption avcodec_options[] = {
>> {"noout", "skip bitstream encoding", 0, AV_OPT_TYPE_CONST, {.i64 =
>> CODEC_FLAG2_NO_OUTPUT }, INT_MIN, INT_MAX, V|E, "flags2"},
>> {"ignorecrop", "ignore cropping information from sps", 1,
>> AV_OPT_TYPE_CONST, {.i64 = CODEC_FLAG2_IGNORE_CROP }, INT_MIN, INT_MAX,
>> V|D, "flags2"},
>> {"local_header", "place global headers at every keyframe instead of in
>> extradata", 0, AV_OPT_TYPE_CONST, {.i64 = CODEC_FLAG2_LOCAL_HEADER },
>> INT_MIN, INT_MAX, V|E, "flags2"},
>> +{"showall", "Show all frames before the first keyframe", 0,
>> AV_OPT_TYPE_CONST, {.i64 = CODEC_FLAG2_SHOW_ALL }, INT_MIN, INT_MAX, V|D,
>> "flags2"},
>> {"me_method", "set motion estimation method", OFFSET(me_method),
>> AV_OPT_TYPE_INT, {.i64 = ME_EPZS }, INT_MIN, INT_MAX, V|E, "me_method"},
>> {"zero", "zero motion estimation (fastest)", 0, AV_OPT_TYPE_CONST, {.i64
>> = ME_ZERO }, INT_MIN, INT_MAX, V|E, "me_method" },
>> {"full", "full motion estimation (slowest)", 0, AV_OPT_TYPE_CONST, {.i64
>> = ME_FULL }, INT_MIN, INT_MAX, V|E, "me_method" },
>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>> index 098bbed..78bad66 100644
>> --- a/libavutil/frame.c
>> +++ b/libavutil/frame.c
>> @@ -379,6 +379,7 @@ int av_frame_copy_props(AVFrame *dst, const AVFrame
>> *src)
>> dst->quality = src->quality;
>> dst->coded_picture_number = src->coded_picture_number;
>> dst->display_picture_number = src->display_picture_number;
>> + dst->flags = src->flags;
>>
>> memcpy(dst->error, src->error, sizeof(dst->error));
>>
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index b0676e7..19fdc1b 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -350,6 +350,9 @@ typedef struct AVFrame {
>>
>> AVFrameSideData **side_data;
>> int nb_side_data;
>> +
>> + int flags;
>> +#define FRAME_FLAG_INCOMPLETE_FRAME 0x0001
>> } AVFrame;
>
> I adding the flag here breaks ABI; this kind on data should go in side_data
> in my opinion. Also you always set this flag but never read it, is this the
> intended behavior?
This is information that may be useful to an application. It is not needed by
libav. It can be eliminated if you all
feel strongly that you don't want it. But I think an application that chooses
to enable the "showall" feature may also
be interested in which frames are actually complete and which are not. I
haven't looked into the design of side_data,
so I don't know if it is appropriate to put this flag there. If someone who
understands the side_data design thinks
this is an architecturally sound thing to do, I would gladly move it there.
--
John GnuPG fingerprint: D0EC B3DB C372 D1F1 0B01 83F0 49F1 D7B2 60D4 D0F7
signature.asc
Description: OpenPGP digital signature
_______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
