On Thu, Oct 10, 2013 at 11:37 PM, John Stebbins <[email protected]> wrote:
> 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.
>>> 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.
Yes, umh so if this info is already in avframe why does it need to be
in mpegvideo?
>
>>
>>
>>> /**
>>> 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.
That's a nice purpose and should be available, as long as it doesn't
break ABI though.
> 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.
It's quite easy, take a look at https://patches.libav.org/patch/42767/
All you need is define a new element in AVFrameSideData
(libavutil/frame.h) and then access it like in the patch above.
Cheers,
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
